-
Notifications
You must be signed in to change notification settings - Fork 15.5k
Reland "[DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)" #165032
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Reland "[DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)" #165032
Changes from all commits
2dce10b
6e90949
413533b
c028e0d
332fa01
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -134,19 +134,29 @@ static void resolveTopLevelMetadata(llvm::Function *Fn, | |
|
|
||
| // Find all llvm.dbg.declare intrinsics and resolve the DILocalVariable nodes | ||
| // they are referencing. | ||
| // | ||
| // Some types may be still unresolved. As they can't be cloned, keep | ||
| // references to the types from the base subprogram. | ||
| // FIXME: As a result, variables of cloned subprogram may refer to local types | ||
| // from base subprogram. In such case, type locality information is damaged. | ||
|
Comment on lines
+140
to
+141
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FIXME is a bit unclear about what should be fixed, and explains more about the problem? Perhaps the explanation should go above, and the FIXME should be more specific about what should be fixed? |
||
| auto PrepareVariableMapping = [&VMap](llvm::DILocalVariable *DILocal) { | ||
| if (DILocal->isResolved()) | ||
| return; | ||
|
|
||
| if (llvm::DIType *Ty = DILocal->getType(); Ty && !Ty->isResolved()) | ||
| VMap.MD()[Ty].reset(Ty); | ||
|
|
||
| DILocal->resolve(); | ||
| }; | ||
|
|
||
| for (auto &BB : *Fn) { | ||
| for (auto &I : BB) { | ||
| for (llvm::DbgVariableRecord &DVR : | ||
| llvm::filterDbgVars(I.getDbgRecordRange())) { | ||
| auto *DILocal = DVR.getVariable(); | ||
| if (!DILocal->isResolved()) | ||
| DILocal->resolve(); | ||
| } | ||
| if (auto *DII = dyn_cast<llvm::DbgVariableIntrinsic>(&I)) { | ||
| auto *DILocal = DII->getVariable(); | ||
| if (!DILocal->isResolved()) | ||
| DILocal->resolve(); | ||
| } | ||
| llvm::filterDbgVars(I.getDbgRecordRange())) | ||
| PrepareVariableMapping(DVR.getVariable()); | ||
|
|
||
| if (auto *DII = dyn_cast<llvm::DbgVariableIntrinsic>(&I)) | ||
| PrepareVariableMapping(DII->getVariable()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // REQUIRES: asserts | ||
| // Should trigger GenerateVarArgsThunk. | ||
| // RUN: %clang_cc1 -O0 -triple riscv64-linux-gnu -debug-info-kind=limited -emit-llvm %s -o - | \ | ||
| // RUN: FileCheck %s | ||
|
|
||
| // RUN: %clang_cc1 -O0 -triple riscv64-linux-gnu -debug-info-kind=limited -emit-obj %s -o - | \ | ||
| // RUN: llvm-dwarfdump --verify - | ||
|
|
||
| // This test checks that the varargs thunk is correctly created if types of | ||
| // DILocalVariables of the base function are unresolved at the cloning time. | ||
|
Comment on lines
+9
to
+10
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you elaborate on the test criteria -- as I understand it this test is to avoid a crash in a tricky situation where types have no full definition? If so, please say that explicitly, and what kind of (I'm thinking ahead to if/when this test fails and someone has to determine the intention behind it). |
||
|
|
||
| typedef signed char __int8_t; | ||
| typedef int BOOL; | ||
| class CMsgAgent; | ||
|
|
||
| class CFs { | ||
| public: | ||
| typedef enum {} CACHE_HINT; | ||
| virtual BOOL ReqCacheHint( CMsgAgent* p_ma, CACHE_HINT hint, ... ) ; | ||
| }; | ||
|
|
||
| typedef struct {} _Lldiv_t; | ||
|
|
||
| class CBdVfs { | ||
| public: | ||
| virtual ~CBdVfs( ) {} | ||
| }; | ||
|
|
||
| class CBdVfsImpl : public CBdVfs, public CFs { | ||
| BOOL ReqCacheHint( CMsgAgent* p_ma, CACHE_HINT hint, ... ); | ||
| }; | ||
|
|
||
| BOOL CBdVfsImpl::ReqCacheHint( CMsgAgent* p_ma, CACHE_HINT hint, ... ) { | ||
| // Complete enum with no defintion. Clang allows to declare variables of | ||
| // such type. | ||
| enum class E : int; | ||
| E enum_var; | ||
| // Structure has forward declaration. DIDerivedType which is a pointer | ||
| // to it is unresolved during debug info generation for the function. | ||
| struct LocalStruct; | ||
| LocalStruct *struct_var; | ||
|
|
||
| struct GlobalStruct {}; | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| // Check that thunk is emitted. | ||
| // CHECK: define {{.*}} @_ZThn{{[48]}}_N10CBdVfsImpl12ReqCacheHintEP9CMsgAgentN3CFs10CACHE_HINTEz( | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,9 +18,9 @@ class B : public A { | |
| static int public_static; | ||
|
|
||
| protected: | ||
| // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_using",{{.*}} line: [[@LINE+3]],{{.*}} flags: DIFlagProtected) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm interested in why this is failing / necessary -- the '-DAG' suffix should mean the check lines are re-orderable anyway, and as far as I read the diff you're only changing the CHECK-line position and updating the relative-line-num calculation accordingly? |
||
| // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_typedef",{{.*}} line: [[@LINE+1]],{{.*}} flags: DIFlagProtected) | ||
| typedef int prot_typedef; | ||
| // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_using",{{.*}} line: [[@LINE+1]],{{.*}} flags: DIFlagProtected) | ||
| using prot_using = prot_typedef; | ||
| prot_using prot_member; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,13 +51,13 @@ void instantiate(int x) { | |
| // CHECK: !DIGlobalVariable(name: "b",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true | ||
| // CHECK: !DIGlobalVariable(name: "result", {{.*}} isLocal: false, isDefinition: true | ||
| // CHECK: !DIGlobalVariable(name: "value", {{.*}} isLocal: false, isDefinition: true | ||
| // CHECK: !DILocalVariable(name: "i", {{.*}}, flags: DIFlagArtificial | ||
| // CHECK: !DILocalVariable(name: "c", {{.*}}, flags: DIFlagArtificial | ||
| // CHECK: !DILocalVariable( | ||
| // CHECK-NOT: name: | ||
| // CHECK: type: ![[UNION:[0-9]+]] | ||
| // CHECK: ![[UNION]] = distinct !DICompositeType(tag: DW_TAG_union_type, | ||
| // CHECK: ![[UNION:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_union_type, | ||
| // CHECK-NOT: name: | ||
| // CHECK: elements | ||
| // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "i", scope: ![[UNION]], | ||
| // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "c", scope: ![[UNION]], | ||
| // CHECK: !DILocalVariable(name: "i", {{.*}}, flags: DIFlagArtificial | ||
| // CHECK: !DILocalVariable(name: "c", {{.*}}, flags: DIFlagArtificial | ||
| // CHECK: !DILocalVariable( | ||
| // CHECK-NOT: name: | ||
| // CHECK: type: ![[UNION]] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be CHECK-SAME, to ensure that we're on the same line as the nameless DILocalVariable? Otherwise this (while unlikely) could match an unrelated later type. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"some" feels concerningly vague - could you be more specific?