[llvm] [DebugInfo] Swap 'Unit' and 'Type' positions in DISubprogram. (PR #96474)

Abid Qadeer via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 24 03:32:57 PDT 2024


https://github.com/abidh created https://github.com/llvm/llvm-project/pull/96474

Due to the current order of metadata in DISubprgram, `Type` is processed before `Unit` by the Verifier. This can cause a race and 
 use of garbage data. Consider the following code:

```
int test(int a[][5])
{
    return a[0][2];
}
```

when compiled with clang, the control reaches `Verifier::visitDISubrange` first with `CurrentSourceLang` still equal to dwarf::DW_LANG_lo_user (32768). The `Verifier::visitDICompileUnit` which sets the value of `CurrentSourceLang` is reached later. So `Verifier::visitDISubrange` ends up using a wrong value of `CurrentSourceLang`.

This behavior does not effect C like language much but is a problem for Fortran. There is special processing in `Verifier::visitDISubrange` when `CurrentSourceLang` is Fortran. With this problem, that special handling is missed and verifier fails for any code that has Fortran's assumed size array in a global subroutine.

To fix this, I have swapped the position of `Type` and `Unit`. They were already adjacent so it does not require changing position of anything else.

>From 41811aa508e208e0f3d96fa2b6fabf9224604596 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Mon, 24 Jun 2024 10:23:23 +0100
Subject: [PATCH] [DebugInfo] Swap 'Unit' and 'Type' positions in DISubprogram.

In current order, `Type` is processed before `Unit` by the Verifier.
This can cause a race condition. Take the following example code:

```
int test(int a[][5])
{
    return a[0][2];
}
```

when compiled with clang, you will notice that control reaches
`Verifier::visitDISubrange` first with `CurrentSourceLang` still equal
to dwarf::DW_LANG_lo_user (32768). The control reaches
`Verifier::visitDICompileUnit` later and sets the value of
`CurrentSourceLang` correctly.

This behavior does not effect C like language much but is a problem for
Fortran. There is special processing in `Verifier::visitDISubrange` when
`CurrentSourceLang` is Fortran. With this problem, that special handling
is missed and verifier fails for any code that has Fortran's assumed
size array in a global subroutine.

To fix this, I have swapped the position of `Type` and `Unit`. They
were already adjacent so it does not require changing position of
anything else.
---
 llvm/include/llvm/IR/DebugInfoMetadata.h | 15 ++++++++-------
 llvm/lib/IR/DebugInfoMetadata.cpp        |  7 +++----
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 524945862e8d4..a1d2f4c1791cf 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1865,6 +1865,11 @@ class DISubprogram : public DILocalScope {
   /// Only used by clients of CloneFunction, and only right after the cloning.
   void replaceLinkageName(MDString *LN) { replaceOperandWith(3, LN); }
 
+  DICompileUnit *getUnit() const {
+    return cast_or_null<DICompileUnit>(getRawUnit());
+  }
+  void replaceUnit(DICompileUnit *CU) { replaceOperandWith(4, CU); }
+
   DISubroutineType *getType() const {
     return cast_or_null<DISubroutineType>(getRawType());
   }
@@ -1873,13 +1878,9 @@ class DISubprogram : public DILocalScope {
   }
   void replaceType(DISubroutineType *Ty) {
     assert(isDistinct() && "Only distinct nodes can mutate");
-    replaceOperandWith(4, Ty);
+    replaceOperandWith(5, Ty);
   }
 
-  DICompileUnit *getUnit() const {
-    return cast_or_null<DICompileUnit>(getRawUnit());
-  }
-  void replaceUnit(DICompileUnit *CU) { replaceOperandWith(5, CU); }
   DITemplateParameterArray getTemplateParams() const {
     return cast_or_null<MDTuple>(getRawTemplateParams());
   }
@@ -1903,8 +1904,8 @@ class DISubprogram : public DILocalScope {
   Metadata *getRawScope() const { return getOperand(1); }
   MDString *getRawName() const { return getOperandAs<MDString>(2); }
   MDString *getRawLinkageName() const { return getOperandAs<MDString>(3); }
-  Metadata *getRawType() const { return getOperand(4); }
-  Metadata *getRawUnit() const { return getOperand(5); }
+  Metadata *getRawUnit() const { return getOperand(4); }
+  Metadata *getRawType() const { return getOperand(5); }
   Metadata *getRawDeclaration() const { return getOperand(6); }
   Metadata *getRawRetainedNodes() const { return getOperand(7); }
   Metadata *getRawContainingType() const {
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index 161a30dfb3828..438ac7b96f345 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1138,10 +1138,9 @@ DISubprogram *DISubprogram::getImpl(
                          RetainedNodes, ThrownTypes, Annotations,
                          TargetFuncName));
   SmallVector<Metadata *, 13> Ops = {
-      File,           Scope,          Name,        LinkageName,
-      Type,           Unit,           Declaration, RetainedNodes,
-      ContainingType, TemplateParams, ThrownTypes, Annotations,
-      TargetFuncName};
+      File,        Scope,       Name,          LinkageName,    Unit,
+      Type,        Declaration, RetainedNodes, ContainingType, TemplateParams,
+      ThrownTypes, Annotations, TargetFuncName};
   if (!TargetFuncName) {
     Ops.pop_back();
     if (!Annotations) {



More information about the llvm-commits mailing list