Fix objc_sel_{name,types} missing an underscore

[Midar]

The other places all use the underscore.

This is just a quick drive-by commit, LMK if more is needed.

[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-clang-codegen

@llvm/pr-subscribers-clang

Author: Jonathan Schleifer (Midar)

Changes

The other places all use the underscore.

This is just a quick drive-by commit, LMK if more is needed.

---

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

1 Files Affected:

• (modified) clang/lib/CodeGen/CGObjCGNU.cpp (+3-2)

diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index 4e7f777ba1d916..97df8c5ae18c64 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -3873,13 +3873,14 @@ llvm::Function *CGObjCGNU::ModuleInitFunction() {
 
     for (auto &untypedSel : allSelectors) {
       std::string selNameStr = untypedSel.getAsString();
-      llvm::Constant *selName = ExportUniqueString(selNameStr, ".objc_sel_name");
+      llvm::Constant *selName = ExportUniqueString(selNameStr,
+        ".objc_sel_name_");
 
       for (TypedSelector &sel : table[untypedSel]) {
         llvm::Constant *selectorTypeEncoding = NULLPtr;
         if (!sel.first.empty())
           selectorTypeEncoding =
-            MakeConstantString(sel.first, ".objc_sel_types");
+            MakeConstantString(sel.first, ".objc_sel_types_");
 
         auto selStruct = selectors.beginStruct(selStructTy);
         selStruct.add(selName);

[Midar]

Maybe @rjmccall would be a good reviewer?

[rjmccall]

At first, I was worried that this was an ABI break. Then I noticed the internal inconsistency within this single file, and so I became worried that it was an ABI fix. But then I noticed that the normal selector-emission code actually makes these strings hidden by default, and so now my guess is that the uniqueness of these strings is at best an optimization. Still, it's an optimization we should do.

There are actually three differences between the code emission here and the code emission in the "primary" paths in GetConstantSelector and GetTypeString:

• the primary path uses an underscore in the symbol name prefix
• the primary path adjusts the symbol name to avoid restricted characters in the target object file format
• the primary path generates hidden symbols

My assumption is that, on all three of these points, we should just do what the primary path would do. Please extract common functions as necessary in order to reuse the logic.

@davidchisnall, could you verify my reasoning here?

[Midar]

These symbols are just used to deduplicate strings: That's why they're hidden and in COMDAT. So I don't think it'll break or fix the ABI ;).

[davidchisnall]

Yup, it’s just an optimisation. The runtime does a slightly more complex deduplication pass during loading, this just does a best effort merge so that we don’t end up with hundreds of copies of common selectors as we did with the GCC ABI. If we don’t do it, it isn’t an ABI break, it’s only an ABI break if we merge two strings that are not the same.

Some of this code accreted and so deduplicating the code, as well as the selectors, is a good idea.

[rjmccall]

Great, thanks! @Midar, if you can fix this code up so that these strings are created in one place (and test the new output, since this is not NFC), I'd be happy to sign off.