Fix globals being wrongly tagged after global optimization step

[tarcisiofischer]

Global tagging for MTE is currently only supported for non-RO data. Most cases are already handled, but some global variables are late marked as RO, and the SanitizerMetadata must be updated.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: Tarcísio Fischer (tarcisiofischer)

Changes

Global tagging for MTE is currently only supported for non-RO data. Most cases are already handled, but some global variables are late marked as RO, and the SanitizerMetadata must be updated.

---

Full diff: https://github.com/llvm/llvm-project/pull/132764.diff

3 Files Affected:

• (modified) llvm/include/llvm/IR/GlobalValue.h (+1)
• (modified) llvm/lib/IR/Globals.cpp (+10)
• (modified) llvm/lib/Transforms/IPO/GlobalOpt.cpp (+4-1)

diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index 2176e2c2cfbfc..b6dd349dd7889 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -362,6 +362,7 @@ class GlobalValue : public Constant {
   void setSanitizerMetadata(SanitizerMetadata Meta);
   void removeSanitizerMetadata();
   void setNoSanitizeMetadata();
+  void disableSanitizerMetadataGlobalTagging();
 
   bool isTagged() const {
     return hasSanitizerMetadata() && getSanitizerMetadata().Memtag;
diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp
index 8ca44719a3f94..8ed46c1d99c25 100644
--- a/llvm/lib/IR/Globals.cpp
+++ b/llvm/lib/IR/Globals.cpp
@@ -266,6 +266,16 @@ void GlobalValue::setNoSanitizeMetadata() {
   setSanitizerMetadata(Meta);
 }
 
+void GlobalValue::disableSanitizerMetadataGlobalTagging() {
+  if (!isTagged()) {
+    return;
+  }
+
+  auto MD = getSanitizerMetadata();
+  MD.Memtag = false;
+  setSanitizerMetadata(MD);
+}
+
 StringRef GlobalObject::getSectionImpl() const {
   assert(hasSection());
   return getContext().pImpl->GlobalObjectSections[this];
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 2d046f09f1b2b..cf2f0f7393531 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -1525,6 +1525,7 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
     if (GS.Ordering == AtomicOrdering::NotAtomic) {
       assert(!GV->isConstant() && "Expected a non-constant global");
       GV->setConstant(true);
+      GV->disableSanitizerMetadataGlobalTagging();
       Changed = true;
     }
 
@@ -2257,8 +2258,10 @@ static bool EvaluateStaticConstructor(Function *F, const DataLayout &DL,
                       << " stores.\n");
     for (const auto &Pair : NewInitializers)
       Pair.first->setInitializer(Pair.second);
-    for (GlobalVariable *GV : Eval.getInvariants())
+    for (GlobalVariable *GV : Eval.getInvariants()) {
       GV->setConstant(true);
+      GV->disableSanitizerMetadataGlobalTagging();
+    }
   }
 
   return EvalSuccess;

[DanielKristofKiss]

the braces are not need for 1 liner condition. ( git clang-format should handle it too)

[fmayer]

I am not opposed to this, but I would prefer a solution that doesn't require cooperation by every callsite.

[fmayer]

I am not opposed to this, but I would prefer a solution that doesn't require cooperation by every callsite.

I am working on a change that does this.

[tarcisiofischer]

I am not opposed to this, but I would prefer a solution that doesn't require cooperation by every callsite.

I am working on a change that does this.

Thanks. Sorry I didn't have the time to get back into this.. My interpretation from what you said was to just move the code to the setter method, instead of calling it every time the set is called, but I didn't come back to make the change and push. I don't mind you tackling it in any way you find more appropriate.

If you do push a new PR, thought, please link it here! :)

[fmayer]

#135891 is my PR for this

[fmayer]

Submitted 9ed4c70 that addresses the same problem in a more general way.