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

snmalloc/0.7.0 package update #35468

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

octo-sts[bot]
Copy link
Contributor

@octo-sts octo-sts bot commented Nov 28, 2024

  • downgrade the clang version to use 17 mentioned in the upstream

  • change the fetch pipeline and use git-checkout instead

  • Add clang-17-dev package for fatal error: 'stddef.h' file not found

@octo-sts octo-sts bot added request-version-update request for a newer version of a package automated pr labels Nov 28, 2024
Copy link
Contributor Author

octo-sts bot commented Nov 28, 2024

Gen AI suggestions to solve the build error:

• Detected Error:

/home/build/src/snmalloc/override/memcpy.cc:8:3: error: attribute declaration must precede definition [-Werror,-Wignored-attributes]
/home/build/src/snmalloc/override/memcpy.cc:9:24: error: redefinition of a 'extern inline' function 'memcpy' is not supported in C++

• Error Category: Build Configuration

• Failure Point: Compilation of memcpy.cc during cmake/build step

• Root Cause Analysis:
The error indicates two issues:

  1. Attribute declaration ordering problem with SNMALLOC_EXPORT
  2. Conflict with existing memcpy definition from glibc headers

• Suggested Fix:
Add the following CMake option to disable memcpy override:

  - uses: cmake/configure
    with:
      opts: |
        -DCMAKE_BUILD_TYPE=Release \
        -DCMAKE_C_COMPILER=clang \
        -DCMAKE_CXX_COMPILER=clang++ \
        -DSNMALLOC_BUILD_TESTING=OFF \
        -DSNMALLOC_LINK_ICF=ON \
        -DSNMALLOC_IPO=ON \
        -DSNMALLOC_STATIC_LIBRARY_PREFIX= \
        -DSNMALLOC_OVERRIDE_MEMCPY=OFF

• Explanation:
The build is failing because snmalloc tries to override the memcpy function, but this conflicts with glibc's fortified memcpy implementation. Disabling the memcpy override through SNMALLOC_OVERRIDE_MEMCPY=OFF will prevent this conflict while maintaining the core allocation functionality.

• Additional Notes:

  • This is a known issue when building with newer glibc versions that provide fortified string functions
  • The memcpy override is not critical for the core allocator functionality
  • This solution maintains compatibility while preserving security features from glibc

• References:

@mamccorm mamccorm force-pushed the wolfictl-00b6bf1e-4106-4278-ab29-2f12c312616d branch from cd291b7 to 4ec111b Compare December 12, 2024 01:41
Copy link
Contributor Author

octo-sts bot commented Dec 12, 2024

Gen AI suggestions to solve the build error:

• Detected Error:

/home/build/src/snmalloc/override/memcpy.cc:8:3: error: attribute declaration must precede definition [-Werror,-Wignored-attributes]
/home/build/src/snmalloc/override/memcpy.cc:9:24: error: redefinition of a 'extern inline' function 'memcpy' is not supported in C++

• Error Category: Build Configuration

• Failure Point: Compilation of memcpy.cc during cmake/build step

• Root Cause Analysis:
The error occurs because:

  1. The visibility attribute is being applied after a previous definition
  2. There's an attempt to redefine memcpy which is already defined inline in glibc headers

• Suggested Fix:
Add a CMake configuration option to disable memcpy override:

  - uses: cmake/configure
    with:
      opts: |
        -DCMAKE_BUILD_TYPE=Release \
        -DCMAKE_C_COMPILER=clang \
        -DCMAKE_CXX_COMPILER=clang++ \
        -DSNMALLOC_BUILD_TESTING=OFF \
        -DSNMALLOC_LINK_ICF=ON \
        -DSNMALLOC_IPO=ON \
        -DSNMALLOC_STATIC_LIBRARY_PREFIX= \
        -DSNMALLOC_OVERRIDE_MEMCPY=OFF

• Explanation:
The build fails because snmalloc tries to override the memcpy function, but this conflicts with glibc's fortified memcpy implementation. Disabling the memcpy override through the CMake option should allow the build to proceed while still maintaining the core allocation functionality.

• Additional Notes:

  • This is a common issue when building memory allocators against glibc
  • The memcpy override is an optimization feature but not critical for core functionality
  • Similar issues have been reported for other allocators trying to override memcpy with glibc

• References:

Copy link
Contributor Author

octo-sts bot commented Dec 14, 2024

Gen AI suggestions to solve the build error:

Based on the build log provided, I'll analyze the CMake configuration failure.

• Detected Error: The build fails during CMake configuration with incomplete compiler ABI detection

• Error Category: Build Configuration

• Failure Point: cmake/configure step during compiler detection

• Root Cause Analysis:
The build stops during CMake's compiler ABI detection phase. This often indicates missing compiler requirements or incorrect compiler settings. In this case, it's likely related to the clang configuration not being properly set up for the build environment.

• Suggested Fix:

  1. Add additional environment variables to control Clang's behavior:
environment:
  contents:
    packages:
      # existing packages...
  env:
    CC: clang
    CXX: clang++
    CFLAGS: "-fPIC"
    CXXFLAGS: "-fPIC"
    LDFLAGS: "-fuse-ld=lld"
  1. Update the cmake/configure opts:
  - uses: cmake/configure
    with:
      opts: |
        -DCMAKE_BUILD_TYPE=Release \
        -DCMAKE_C_COMPILER=clang-18 \
        -DCMAKE_CXX_COMPILER=clang++-18 \
        -DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=lld" \
        -DSNMALLOC_BUILD_TESTING=OFF \
        -DSNMALLOC_LINK_ICF=ON \
        -DSNMALLOC_IPO=ON \
        -DSNMALLOC_STATIC_LIBRARY_PREFIX=

• Explanation:

  • The build is failing during compiler ABI detection, suggesting Clang isn't properly configured
  • Adding explicit compiler environment variables ensures consistent compiler selection
  • Using version-specific compiler names (clang-18) matches the installed package
  • The -fPIC flag ensures position-independent code generation
  • Using lld as the linker can help with modern linking requirements

• Additional Notes:

  • snmalloc is sensitive to compiler and linker configurations
  • The project requires modern C++ features supported by Clang 18
  • Position-independent code is important for shared library builds

• References:

@debasishbsws debasishbsws self-assigned this Dec 17, 2024
…stream https://github.com/microsoft/snmalloc/blob/main/docs/BUILDING.md\#building-on-unix-like-platforms

Update the fetch pipeline to use git-checkout

Add clang dev package for fatal error: 'stddef.h' file not found

Signed-off-by: debasishbsws <[email protected]>
@octo-sts octo-sts bot added the bincapz/pass bincapz/pass Bincapz (aka. malcontent) scan didn't detect any CRITICALs on the scanned packages. label Dec 17, 2024
@debasishbsws debasishbsws requested a review from a team December 17, 2024 12:52
snmalloc.yaml Show resolved Hide resolved
@debasishbsws
Copy link
Member

with clang or llvm version 18 we started getting an error redefinition of a 'extern inline' function 'memcpy' is not supported in C++

Cause according to claude AI

Root Cause
- LLVM 18 is more strict about attribute placement and function redefinitions
- The issue appears to be with memcpy redefinition in snmalloc/override/memcpy.cc
- The SNMALLOC_EXPORT attribute is being applied after a previous definition

it suggested me to do some code changes. I started looking into upstream guide and it suggested dependencies: CMake, Ninja, Clang 6.0 or later and a C++17 standard library.

I try to downgrade the version to suggested 17 and it worked.

Copy link
Member

@luhring luhring left a comment

Choose a reason for hiding this comment

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

Thanks!!

@luhring luhring enabled auto-merge December 18, 2024 14:28
@luhring luhring merged commit 39cdc18 into main Dec 18, 2024
13 checks passed
@luhring luhring deleted the wolfictl-00b6bf1e-4106-4278-ab29-2f12c312616d branch December 18, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automated pr bincapz/pass bincapz/pass Bincapz (aka. malcontent) scan didn't detect any CRITICALs on the scanned packages. request-version-update request for a newer version of a package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants