Skip to content

Revert "[IR] Avoid UB in SymbolTableListTraits" #142093

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

Merged
merged 1 commit into from
May 30, 2025

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented May 30, 2025

Reverts #139096 due to invalid uses of offsetof on non-standard-layout types.

@dtcxzyw dtcxzyw requested review from hahnjo and Michael137 May 30, 2025 06:48
@llvmbot
Copy link
Member

llvmbot commented May 30, 2025

@llvm/pr-subscribers-llvm-ir

Author: Yingwei Zheng (dtcxzyw)

Changes

Reverts llvm/llvm-project#139096 due to invalid uses of offsetof on non-standard-layout types.


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

4 Files Affected:

  • (modified) llvm/include/llvm/IR/BasicBlock.h (-4)
  • (modified) llvm/include/llvm/IR/Function.h (-4)
  • (modified) llvm/include/llvm/IR/Module.h (-12)
  • (modified) llvm/include/llvm/IR/SymbolTableListTraits.h (+4-2)
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index 10617db09fde6..9ee0bacb5c258 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -546,10 +546,6 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
     return &BasicBlock::InstList;
   }
 
-  static size_t getSublistOffset(Instruction *) {
-    return offsetof(BasicBlock, InstList);
-  }
-
   /// Dedicated function for splicing debug-info: when we have an empty
   /// splice (i.e. zero instructions), the caller may still intend any
   /// debug-info in between the two "positions" to be spliced.
diff --git a/llvm/include/llvm/IR/Function.h b/llvm/include/llvm/IR/Function.h
index 63100568d07e4..6d4a53da7ff22 100644
--- a/llvm/include/llvm/IR/Function.h
+++ b/llvm/include/llvm/IR/Function.h
@@ -811,10 +811,6 @@ class LLVM_ABI Function : public GlobalObject, public ilist_node<Function> {
     return &Function::BasicBlocks;
   }
 
-  static size_t getSublistOffset(BasicBlock *) {
-    return offsetof(Function, BasicBlocks);
-  }
-
 public:
   const BasicBlock       &getEntryBlock() const   { return front(); }
         BasicBlock       &getEntryBlock()         { return front(); }
diff --git a/llvm/include/llvm/IR/Module.h b/llvm/include/llvm/IR/Module.h
index a305dd99962c0..0dc8164b9c950 100644
--- a/llvm/include/llvm/IR/Module.h
+++ b/llvm/include/llvm/IR/Module.h
@@ -608,9 +608,6 @@ class LLVM_ABI Module {
   static GlobalListType Module::*getSublistAccess(GlobalVariable*) {
     return &Module::GlobalList;
   }
-  static size_t getSublistOffset(GlobalVariable *) {
-    return offsetof(Module, GlobalList);
-  }
   friend class llvm::SymbolTableListTraits<llvm::GlobalVariable>;
 
 public:
@@ -621,9 +618,6 @@ class LLVM_ABI Module {
   static FunctionListType Module::*getSublistAccess(Function*) {
     return &Module::FunctionList;
   }
-  static size_t getSublistOffset(Function *) {
-    return offsetof(Module, FunctionList);
-  }
 
   /// Detach \p Alias from the list but don't delete it.
   void removeAlias(GlobalAlias *Alias) { AliasList.remove(Alias); }
@@ -663,9 +657,6 @@ class LLVM_ABI Module {
   static AliasListType Module::*getSublistAccess(GlobalAlias*) {
     return &Module::AliasList;
   }
-  static size_t getSublistOffset(GlobalAlias *) {
-    return offsetof(Module, AliasList);
-  }
   friend class llvm::SymbolTableListTraits<llvm::GlobalAlias>;
 
   /// Get the Module's list of ifuncs (constant).
@@ -676,9 +667,6 @@ class LLVM_ABI Module {
   static IFuncListType Module::*getSublistAccess(GlobalIFunc*) {
     return &Module::IFuncList;
   }
-  static size_t getSublistOffset(GlobalIFunc *) {
-    return offsetof(Module, IFuncList);
-  }
   friend class llvm::SymbolTableListTraits<llvm::GlobalIFunc>;
 
   /// Get the Module's list of named metadata (constant).
diff --git a/llvm/include/llvm/IR/SymbolTableListTraits.h b/llvm/include/llvm/IR/SymbolTableListTraits.h
index 456833fff62ce..fcf6f0fb15280 100644
--- a/llvm/include/llvm/IR/SymbolTableListTraits.h
+++ b/llvm/include/llvm/IR/SymbolTableListTraits.h
@@ -77,8 +77,10 @@ class SymbolTableListTraits : public ilist_alloc_traits<ValueSubClass> {
   /// getListOwner - Return the object that owns this list.  If this is a list
   /// of instructions, it returns the BasicBlock that owns them.
   ItemParentClass *getListOwner() {
-    size_t Offset = ItemParentClass::getSublistOffset(
-        static_cast<ValueSubClass *>(nullptr));
+    size_t Offset = reinterpret_cast<size_t>(
+        &((ItemParentClass *)nullptr->*ItemParentClass::getSublistAccess(
+                                           static_cast<ValueSubClass *>(
+                                               nullptr))));
     ListTy *Anchor = static_cast<ListTy *>(this);
     return reinterpret_cast<ItemParentClass*>(reinterpret_cast<char*>(Anchor)-
                                               Offset);

Copy link
Contributor

@kazutakahirata kazutakahirata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@dtcxzyw dtcxzyw merged commit aa7d7b3 into main May 30, 2025
8 of 12 checks passed
@dtcxzyw dtcxzyw deleted the revert-139096-perf/safe_container_of branch May 30, 2025 06:53
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
Reverts llvm#139096 due to invalid uses of `offsetof` on
non-standard-layout types.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants