Skip to content

Conversation

xingxue-ibm
Copy link
Contributor

Some libc++ LIT test cases and user code define their own version of operator delete that are not sized. With -fno-sized-deallocation, destructors call the non-sized operator delete and it will be resolved to the user defined version. However, with -fsized-deallocation, destructors will call the sized operator delete which will be resolved to the weak definition in libc++abi because the user code does not define the corresponding sized version. The libc++abi sized operator delete in turn calls the non-sized version of operator delete of the same shared object inside libc++abi instead of the user defined version on AIX because runtime linking is not the default for AIX and therefore, fails the tests or user code. This patch sets -fno-sized-deallocation as the default for AIX if neither -fsize-deallocation nor -fno-sized-deallocation is explicitly set, similar to what is done for ZOS.

@xingxue-ibm xingxue-ibm added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jun 28, 2024
@xingxue-ibm xingxue-ibm self-assigned this Jun 28, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 28, 2024

@llvm/pr-subscribers-backend-powerpc
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Xing Xue (xingxue-ibm)

Changes

Some libc++ LIT test cases and user code define their own version of operator delete that are not sized. With -fno-sized-deallocation, destructors call the non-sized operator delete and it will be resolved to the user defined version. However, with -fsized-deallocation, destructors will call the sized operator delete which will be resolved to the weak definition in libc++abi because the user code does not define the corresponding sized version. The libc++abi sized operator delete in turn calls the non-sized version of operator delete of the same shared object inside libc++abi instead of the user defined version on AIX because runtime linking is not the default for AIX and therefore, fails the tests or user code. This patch sets -fno-sized-deallocation as the default for AIX if neither -fsize-deallocation nor -fno-sized-deallocation is explicitly set, similar to what is done for ZOS.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/AIX.cpp (+6)
  • (modified) clang/unittests/StaticAnalyzer/CallEventTest.cpp (+4)
diff --git a/clang/lib/Driver/ToolChains/AIX.cpp b/clang/lib/Driver/ToolChains/AIX.cpp
index 381d72e045b95..b04502a57a9f7 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -551,6 +551,12 @@ void AIX::addClangTargetOptions(
   if (Args.hasFlag(options::OPT_fxl_pragma_pack,
                    options::OPT_fno_xl_pragma_pack, true))
     CC1Args.push_back("-fxl-pragma-pack");
+
+  // Pass "-fno-sized-deallocation" only when the user hasn't manually enabled
+  // or disabled sized deallocations.
+  if (!Args.getLastArgNoClaim(options::OPT_fsized_deallocation,
+                              options::OPT_fno_sized_deallocation))
+    CC1Args.push_back("-fno-sized-deallocation");
 }
 
 void AIX::addProfileRTLibs(const llvm::opt::ArgList &Args,
diff --git a/clang/unittests/StaticAnalyzer/CallEventTest.cpp b/clang/unittests/StaticAnalyzer/CallEventTest.cpp
index 7c4132788ca7e..de28bb158ef66 100644
--- a/clang/unittests/StaticAnalyzer/CallEventTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallEventTest.cpp
@@ -76,7 +76,11 @@ TEST(CXXDeallocatorCall, SimpleDestructor) {
     }
   )",
                                                          Diags));
+#if !defined(__cpp_sized_deallocation)
+  EXPECT_EQ(Diags, "test.CXXDeallocator: NumArgs: 1\n");
+#else
   EXPECT_EQ(Diags, "test.CXXDeallocator: NumArgs: 2\n");
+#endif
 }
 
 } // namespace

…ation because the latter is not defined for the test.
Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

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

LGTM.

@xingxue-ibm xingxue-ibm merged commit 668ee3f into llvm:main Jul 1, 2024
cpiaseque pushed a commit to cpiaseque/llvm-project that referenced this pull request Jul 3, 2024
Some `libc++` LIT test cases and user code define their own version of
`operator delete` that are not sized. With `-fno-sized-deallocation`,
destructors call the non-sized `operator delete` and it will be resolved
to the user defined version. However, with `-fsized-deallocation`,
destructors will call the sized `operator delete` which will be resolved
to the weak definition in `libc++abi` because the user code does not
define the corresponding sized version. The `libc++abi` sized `operator
delete` in turn calls the non-sized version of `operator delete` of the
same shared object inside `libc++abi` instead of the user defined
version on AIX because runtime linking is not the default for AIX and
therefore, fails the tests or user code. This patch sets
`-fno-sized-deallocation` as the default for AIX if neither
`-fsize-deallocation` nor `-fno-sized-deallocation` is explicitly set,
similar to what is done for ZOS.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
Some `libc++` LIT test cases and user code define their own version of
`operator delete` that are not sized. With `-fno-sized-deallocation`,
destructors call the non-sized `operator delete` and it will be resolved
to the user defined version. However, with `-fsized-deallocation`,
destructors will call the sized `operator delete` which will be resolved
to the weak definition in `libc++abi` because the user code does not
define the corresponding sized version. The `libc++abi` sized `operator
delete` in turn calls the non-sized version of `operator delete` of the
same shared object inside `libc++abi` instead of the user defined
version on AIX because runtime linking is not the default for AIX and
therefore, fails the tests or user code. This patch sets
`-fno-sized-deallocation` as the default for AIX if neither
`-fsize-deallocation` nor `-fno-sized-deallocation` is explicitly set,
similar to what is done for ZOS.
@xingxue-ibm xingxue-ibm deleted the no-sized-deallocation branch April 1, 2025 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:PowerPC clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants