Skip to content
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

Asm code is not compiling under Xcode #3622

Closed
cfnptr opened this issue Apr 18, 2023 · 5 comments · Fixed by #3665
Closed

Asm code is not compiling under Xcode #3622

cfnptr opened this issue Apr 18, 2023 · 5 comments · Fixed by #3665

Comments

@cfnptr
Copy link

cfnptr commented Apr 18, 2023

I have added zstd library to my project as subdirectory using CMake. And when I configure and try to compile using Xcode it shows error in huff_decompress asm code. Problem appeared from 1.5.4 release version.

Steps to reproduce the behavior:

  1. Add zstd as subdirectory in CMake
  2. Configure Xcode project (cmake -G Xcode...)
  3. Open configured .xcodeproj and start program compilation
  4. See error in huf_decompress_amd64.S "Expected identifier or '('" at line 37

Expected behavior:
Program compiling without errors.

Desktop:

  • OS: macOS
  • Version Ventura 13.3.1
  • Compiler clang Xcode 14.3
  • Flags -O0
  • Build system CMake
@sauclovian
Copy link

This is caused by an assembler directive being outside the ifdef. Moving it inside the ifdef will fix the problem. (Change is trivial.) See https://gnats.netbsd.org/57383

@Cyan4973
Copy link
Contributor

This is caused by an assembler directive being outside the ifdef. Moving it inside the ifdef will fix the problem.

The statement is :

/* Stack marking
 * ref: https://wiki.gentoo.org/wiki/Hardened/GNU_stack_quickstart
 */
#if defined(__ELF__) && defined(__GNUC__)
.section .note.GNU-stack,"",%progbits
#endif

This statement is meant to avoid executable stacks, which is considered harmful in certain security contexts.
It must be set, even when the target architecture is not x64, and there is consequently no ASM code to compile.

This is a complex situation, where some environments require this statement, while others do not.

Note that the statement is not unconditional : #if defined(__ELF__) && defined(__GNUC__).
So a strategy could be to extend the nb of conditions to validate, in order to exclude specific environments that do not like this statement.

cc @terrelln.

@gjasny
Copy link
Contributor

gjasny commented May 22, 2023

The reason the build fails (on my machine) is that Xcode adds a -x c to the clang command line.
It does that because CMake adds the following line to the Xcode project:

/* .../zstd-1.5.5/lib/decompress/huf_decompress_amd64.S */ = {isa = PBXFileReference; explicitFileType = sourcecode.c.c; fileEncoding = 4; name = huf_decompress_amd64.S; path = ".../zstd-1.5.5/lib/decompress/huf_decompress_amd64.S"; sourceTree = SOURCE_ROOT; };

The explicitFileType = sourcecode.c.c is the problem. It should be sourcecode.asm and CMake is supposed to select it for assembly files:

https://github.com/Kitware/CMake/blob/dabba15172012f4497c96eadbf69684fc2f597ee/Source/cmGlobalXCodeGenerator.cxx#L1136-L1137

Now I wonder why huf_decompress_amd64.S is not recognized as "Assembly". The innermost project call explicitly enables the ASM language.

I'll try to dig deeper in the next days.

gjasny added a commit to gjasny/zstd that referenced this issue Jun 2, 2023
When forcing the source file language to `C`, Xcode enforces
the file to be compiled as `C` by appending `-x c` to the
compiler command line.

For now try to limit the damage and only enforce the language
if the ASM and C compilers differ.

Reproducer (CMake `3.26.4`, Xcode `14.3`):
```
cmake -S build/cmake -B _b -GXcode -DCMAKE_OSX_ARCHITECTURES=x86_64
cmake --build _b
```

Fix: facebook#3622
@gjasny
Copy link
Contributor

gjasny commented Jun 2, 2023

This gets broken by

# Explicitly set the language to C for all files, including ASM files.
# Our assembly expects to be compiled by a C compiler, and is only enabled for
# __GNUC__ compatible compilers. Otherwise all the ASM code is disabled by
# macros.
set_source_files_properties(${Sources} PROPERTIES LANGUAGE C)

introduced by @terrelln in 651a381 for #3193

My proposal to fix it is in #3665.

gjasny added a commit to gjasny/ccache that referenced this issue Jun 2, 2023
Reproducer (CMake `3.26.4`, Xcode `14.3`):
```
cmake -B _build -GXcode -DCMAKE_OSX_ARCHITECTURES=x86_64 -DZSTD_FROM_INTERNET=ON -DREDIS_STORAGE_BACKEND=OFF
cmake --build _build

```

See facebook/zstd#3622
jrosdahl pushed a commit to ccache/ccache that referenced this issue Jun 7, 2023
Reproducer (CMake 3.26.4, Xcode 14.3):

    cmake -B _build -GXcode -DCMAKE_OSX_ARCHITECTURES=x86_64 \
          -DZSTD_FROM_INTERNET=ON -DREDIS_STORAGE_BACKEND=OFF
    cmake --build _build

See facebook/zstd#3622
gcflymoto pushed a commit to gcflymoto/zstd that referenced this issue Nov 2, 2023
When forcing the source file language to `C`, Xcode enforces
the file to be compiled as `C` by appending `-x c` to the
compiler command line.

For now try to limit the damage and only enforce the language
if the ASM and C compilers differ.

Reproducer (CMake `3.26.4`, Xcode `14.3`):
```
cmake -S build/cmake -B _b -GXcode -DCMAKE_OSX_ARCHITECTURES=x86_64
cmake --build _b
```

Fix: facebook#3622
hswong3i pushed a commit to alvistack/facebook-zstd that referenced this issue Mar 27, 2024
When forcing the source file language to `C`, Xcode enforces
the file to be compiled as `C` by appending `-x c` to the
compiler command line.

For now try to limit the damage and only enforce the language
if the ASM and C compilers differ.

Reproducer (CMake `3.26.4`, Xcode `14.3`):
```
cmake -S build/cmake -B _b -GXcode -DCMAKE_OSX_ARCHITECTURES=x86_64
cmake --build _b
```

Fix: facebook#3622
@matthewbloch
Copy link

matthewbloch commented Jun 21, 2024

This doesn't need reopening, but I hit the exact same issue compiling with the (proprietary) Sony PS5 tool chain. Commenting out the set_source_files_properties line fixes the build (thanks for pointing this out @gjasny), but I don't understand the the build well enough to suggest a long-term fix.

(NB the source comment says this will disable the use of assembler)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants