Skip to content

Commit 5d624fd

Browse files
committed
chore: extract all type properties instead of distinct function check
In Variable.create to check properties of type were used many separate function, but this is not very convenient, because they performed many redundant checks and (what is even worse) if we wanted to perform some small changes in method of 1 debugger class then the whole sequence may be disrupted and we have to change it, but it breaks checks of another. This patch adds single entrypoint for extracting information about variable - IDebuggerFacade.extractVariableProperties, which just returns special object with all extracted knowledge of the given var.
1 parent ccb1cc5 commit 5d624fd

File tree

2 files changed

+250
-65
lines changed

2 files changed

+250
-65
lines changed

src/debugger.ts

Lines changed: 162 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,79 @@ import { Features } from './configuration';
44
import { PgVariablesViewProvider } from './variables';
55
import {EvaluationError} from './error';
66

7+
/* Simple representation of a variable obtained from debugger */
78
export interface IDebugVariable {
89
value: string;
910
type: string;
1011
memoryReference?: string;
1112
}
1213

14+
/* Represent properties of some variable */
15+
export interface IVariableProperties {
16+
/*
17+
* Type is a pointer, i.e. 'int *'
18+
*
19+
* NOTE: it does not check pointer validity.
20+
*/
21+
isPointer(): boolean;
22+
/* Pointer value is NOT NULL, but represents invalid value */
23+
pointerIsInvalid(): boolean;
24+
/* Pointer value is NULL */
25+
pointerIsNull(): boolean;
26+
/* Shortcut for `!(pointerIsNull() || pointerIsInvalid())` */
27+
pointerCanDeref(): boolean;
28+
29+
/* Value struct - structure type without pointers, i.e. */
30+
isValueStruct(): boolean;
31+
/* Value is a scalar (integer, char, etc), i.e. int */
32+
isScalar(): boolean;
33+
/* Type is a fixed size array, i.e. int[16] */
34+
isFixedSizeArray(): boolean;
35+
/* Type is a flexible array member, i.e. int[] */
36+
isFlexibleArray(): boolean;
37+
}
38+
39+
enum TypeProperty {
40+
Pointer = 1 << 0,
41+
PointerNull = 1 << 1,
42+
PointerInvalid = 1 << 2,
43+
ValueStruct = 1 << 3,
44+
Scalar = 1 << 4,
45+
FixedSizeArray = 1 << 5,
46+
FlexibleArray = 1 << 6,
47+
48+
PointerMask = Pointer | PointerNull | PointerInvalid,
49+
}
50+
51+
/* Basic implementation, that use single enum bitmask field for space efficiency */
52+
class TypeProperties implements IVariableProperties {
53+
constructor(private props: TypeProperty) { }
54+
isPointer(): boolean {
55+
return (this.props & TypeProperty.PointerMask) !== 0;
56+
}
57+
pointerIsInvalid(): boolean {
58+
return (this.props & TypeProperty.PointerInvalid) !== 0;
59+
}
60+
pointerIsNull(): boolean {
61+
return (this.props & TypeProperty.PointerNull) !== 0;
62+
}
63+
pointerCanDeref(): boolean {
64+
return (this.props & TypeProperty.Pointer) !== 0;
65+
}
66+
isValueStruct(): boolean {
67+
return (this.props & TypeProperty.ValueStruct) !== 0;
68+
}
69+
isScalar(): boolean {
70+
return (this.props & TypeProperty.Scalar) !== 0;
71+
}
72+
isFixedSizeArray(): boolean {
73+
return (this.props & TypeProperty.FixedSizeArray) !== 0;
74+
}
75+
isFlexibleArray(): boolean {
76+
return (this.props & TypeProperty.FlexibleArray) !== 0;
77+
}
78+
}
79+
1380
const pointerRegex = /^0x[0-9abcdef]+$/i;
1481
const nullRegex = /^0x0+$/i;
1582
const builtInTypes = new Set([
@@ -224,6 +291,8 @@ export interface IDebuggerFacade {
224291
* Get pointer for location of this variable
225292
*/
226293
getPointer: (variable: IDebugVariable) => string | undefined;
294+
/* Extract information about type of this variable */
295+
extractVariableProperties: (dv: IDebugVariable) => IVariableProperties;
227296

228297
/**
229298
*
@@ -582,19 +651,33 @@ export abstract class GenericDebuggerFacade implements IDebuggerFacade, vscode.D
582651
abstract maybeCalcFrameIndex(frameId: number): number | undefined;
583652
abstract shouldShowScope(scope: dap.Scope): boolean;
584653
abstract evaluate(expression: string, frameId: number | undefined, context?: string): Promise<dap.EvaluateResponse>;
654+
abstract extractVariableProperties(dv: IDebugVariable): IVariableProperties;
585655
abstract isNull(variable: IDebugVariable | dap.EvaluateResponse): boolean;
586656
abstract isValidPointerType(variable: IDebugVariable | dap.EvaluateResponse): boolean;
587657
abstract isValueStruct(variable: IDebugVariable, type?: string): boolean;
588658
abstract extractString(variable: IDebugVariable | dap.EvaluateResponse): string | null;
589659
abstract extractBool(variable: IDebugVariable | dap.EvaluateResponse): boolean | null;
590660
abstract extractPtrFromString(variable: IDebugVariable | dap.EvaluateResponse): string | null;
591-
abstract formatEnumValue(name: string, value: string): string;
592661
abstract extractLongString(variable: IDebugVariable, frameId: number): Promise<string | null>;
662+
abstract formatEnumValue(name: string, value: string): string;
663+
}
664+
665+
function pointerValueLooksCorrect(pointer: number) {
666+
/*
667+
* Even if this is pointer it can have garbage. To check this
668+
* compare with some definitely impossible pointer value.
669+
* This can happen not only for garbage, but also when integer
670+
* is assigned to pointer type.
671+
*
672+
* NOTE: this done primarily for cppdbg, because CodeLLDB checks
673+
* pointer correctness.
674+
*/
675+
return Number.isInteger(pointer) && pointer > 0x10000;
593676
}
594677

595678
export class CppDbgDebuggerFacade extends GenericDebuggerFacade {
596679
type = DebuggerType.CppDbg;
597-
680+
598681
shouldShowScope(scope: dap.Scope): boolean {
599682
return scope.name === 'Locals';
600683
}
@@ -683,7 +766,7 @@ export class CppDbgDebuggerFacade extends GenericDebuggerFacade {
683766
* is assigned to pointer type.
684767
*/
685768
const ptrNumber = Number(pointer);
686-
if (Number.isNaN(ptrNumber) || ptrNumber < 0x10000) {
769+
if (!pointerValueLooksCorrect(ptrNumber)) {
687770
return false;
688771
}
689772

@@ -704,6 +787,41 @@ export class CppDbgDebuggerFacade extends GenericDebuggerFacade {
704787
return false;
705788
}
706789

790+
extractVariableProperties(dv: IDebugVariable) {
791+
const value = this.getValue(dv);
792+
793+
let prop: TypeProperty;
794+
if (value.length === 0) {
795+
if (dv.type.endsWith('[]')) {
796+
prop = TypeProperty.FlexibleArray;
797+
} else {
798+
prop = TypeProperty.ValueStruct;
799+
}
800+
} else if (value.startsWith('0x')) {
801+
/* Check for pointer type first - cppdbg always shows pointers in the value field */
802+
const pointerValue = Number(value);
803+
if (!pointerValueLooksCorrect(pointerValue)) {
804+
if (pointerValue === 0) {
805+
prop = TypeProperty.PointerNull;
806+
} else {
807+
prop = TypeProperty.PointerInvalid;
808+
}
809+
} else {
810+
prop = TypeProperty.Pointer;
811+
}
812+
} else if (value === '{...}') {
813+
prop = TypeProperty.ValueStruct;
814+
} else if (dv.type.endsWith(']')) {
815+
/* FLA was checked at first 'if' branch - it has empty 'value' */
816+
prop = TypeProperty.FixedSizeArray;
817+
} else {
818+
/* The only left is a scalar */
819+
prop = TypeProperty.Scalar;
820+
}
821+
822+
return new TypeProperties(prop);
823+
}
824+
707825
extractString(variable: IDebugVariable | dap.EvaluateResponse) {
708826
const value = this.getValue(variable);
709827
const left = value.indexOf('"');
@@ -940,6 +1058,47 @@ export class CodeLLDBDebuggerFacade extends GenericDebuggerFacade {
9401058
throw err;
9411059
}
9421060
}
1061+
1062+
extractVariableProperties(dv: IDebugVariable) {
1063+
let prop: TypeProperty;
1064+
if (dv.value === '<null>' || dv.memoryReference === '0x0') {
1065+
prop = TypeProperty.PointerNull;
1066+
} else if (dv.value === '<invalid pointer>') {
1067+
prop = TypeProperty.PointerInvalid;
1068+
} else if (dv.value.startsWith('0x')) {
1069+
const pointer = Number(dv.value);
1070+
if (!pointerValueLooksCorrect(pointer)) {
1071+
prop = TypeProperty.PointerInvalid;
1072+
} else {
1073+
prop = TypeProperty.Pointer;
1074+
}
1075+
} else if (dv.type.endsWith(']')) {
1076+
/*
1077+
* Array must be checked before structure, because
1078+
* array is rendered like '{ELEM1, ELEM2, ...}'
1079+
*/
1080+
if (dv.type[dv.type.length - 2] === '[') {
1081+
prop = TypeProperty.FlexibleArray;
1082+
} else {
1083+
prop = TypeProperty.FixedSizeArray;
1084+
}
1085+
} else if ((dv.value.startsWith('{') && dv.value.endsWith('}')) || dv.type.indexOf('*') !== -1) {
1086+
/*
1087+
* CodeLLDB is smart and shows contents of structures explicitly,
1088+
* but because PostgreSQL has lots of typedefs to pointers
1089+
* (i.e. 'Data *' suffixes) we just can not tell the difference
1090+
* between Value Struct and Pointer if it's type is a pointer typedef.
1091+
* But we should not assign ValueStruct in such case, because
1092+
* many type-specific properties will not be checked, so assign
1093+
* Pointer type to every structure.
1094+
*/
1095+
prop = TypeProperty.Pointer;
1096+
} else {
1097+
prop = TypeProperty.Scalar;
1098+
}
1099+
1100+
return new TypeProperties(prop);
1101+
}
9431102

9441103
isNull(variable: IDebugVariable | dap.EvaluateResponse): boolean {
9451104
if ('result' in variable) {

0 commit comments

Comments
 (0)