Skip to content

[clang] Default to -fno-sized-deallocation for AIX #97076

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 2 commits into from
Jul 1, 2024

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
5 of 7 checks passed
lravenclaw pushed a commit to lravenclaw/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