[lldb][AIX] Introducing _ALL_SOURCE macro into driver CMakeLists

[DhruvSrivastavaX]

This PR is in reference to porting LLDB on AIX.

Link to discussions on llvm discourse and github:

1. https://discourse.llvm.org/t/port-lldb-to-ibm-aix/80640
2. Extending LLDB to work on AIX #101657
   The complete changes for porting are present in this draft PR:
   Extending LLDB to work on AIX #102601

Adding changes for minimal build for lldb binary on AIX:
The struct winsize needed by lldb/tools/driver/Driver.cpp is only recognised in AIX under the AIX specific _ALL_SOURCE macro, hence its enablement is required here.

Review Request: @labath @DavidSpickett

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Dhruv Srivastava (DhruvSrivastavaX)

Changes

This PR is in reference to porting LLDB on AIX.

Link to discussions on llvm discourse and github:

1. https://discourse.llvm.org/t/port-lldb-to-ibm-aix/80640
2. Extending LLDB to work on AIX #101657
   The complete changes for porting are present in this draft PR:
   Extending LLDB to work on AIX #102601

Adding changes for minimal build for lldb binary on AIX:
The struct winsize needed by lldb/tools/driver/Driver.cpp is only recognised in AIX under the AIX specific ALL_SOURCE macro, hence its enablement is required in certain places.

Review Request: @labath @DavidSpickett

---

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

1 Files Affected:

• (modified) lldb/tools/driver/CMakeLists.txt (+5)

diff --git a/lldb/tools/driver/CMakeLists.txt b/lldb/tools/driver/CMakeLists.txt
index cd304a047dea6d..89884ecd0601bc 100644
--- a/lldb/tools/driver/CMakeLists.txt
+++ b/lldb/tools/driver/CMakeLists.txt
@@ -11,6 +11,11 @@ if(APPLE)
   set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-sectcreate,__TEXT,__info_plist,${CMAKE_CURRENT_BINARY_DIR}/lldb-Info.plist")
 endif()
 
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")                               
+  remove_definitions("-D_XOPEN_SOURCE=700")                                    
+  add_definitions("-D_ALL_SOURCE")                                             
+endif()
+
 add_lldb_tool(lldb
   Driver.cpp
   Platform.cpp

[DavidSpickett]

I assume the macro is _ALL_SOURCE with the leading underscore, you put ALL_SOURCE in the title and description.

I see that

llvm-project/third-party/unittest/CMakeLists.txt

Line 18 in d66f653

# Google Test requires headers which need _ALL_SOURCE to build on AIX

also does this.

llvm-project/clang/tools/libclang/CMakeLists.txt

Line 132 in d66f653

if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")

says it does the same thing but it doesn't add _ALL_SOURCE it only removes the other macro.

The one in third-party seems more logical and matches the one you're adding, so this is fine.

[DavidSpickett]

LGTM.

Please correct the macro name in the title/description then I'll merge this.

[DhruvSrivastavaX]

Hi @DavidSpickett , sure, updated it. Thanks!

[labath]

This is fairly fragile. It may be better to do a -U_XOPEN_SOURCE.

Some comment about why you're undoing the changes here might be in order as well.

[labath]

It may be better to do a -U_XOPEN_SOURCE

.. which, at least according to my experiments, isn't possible it seems. However, also according to my experiments, remove_definitions doesn't actually remove macros defined by add_compile_definitions (only add_definitions), which brings up another question: Is this actually needed? Would it be sufficient to define _ALL_SOURCE as well?

[DhruvSrivastavaX]

Okay, It was added by another team in IBM but I think the intention to add this way was to only utilise the relevant posix standard definitions and avoid any extra un-needed definitions. But then again, some definitions being used in LLDB code, do not come under the _X_OPEN_SOURCE for AIX, in which case we need to use _ALL_SOURCE instead. But yes, I can check if only having add_definitions does not cause any compilation conflicts and update accordingly.

[labath]

Thanks. The way I'd start is the see if the remove_definitions call is actually doing something (in my experiments, it didn't, but I only tried it on toy programs). If it isn't, then you already have both of these defined, and there's nothing further to check.

That said, if you can get it touch with them, it might be good to check they were trying to achieve, as having different definition in different parts of the program is a recipe for problems. From that comment, it sounds like they had problems building llvm with _ALL_SOURCE, but adding the definition here still requires llvm (at least, parts included by this code) to build in this mode.