lib: libbacktrace: update to b9e4006 #11411
Conversation
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
edsiper
requested review from
celalettin1286,
cosmo0920,
niedbalski and
patrick-stephens
as code owners
📝 WalkthroughWalkthroughReplaces inline libbacktrace handling in CMake with an included cmake/libbacktrace.cmake module; adds a new libbacktrace CMake/autotools integration and expands vendored libbacktrace snapshot with platform, address-base, compressed-debug, zstd, Windows, and test changes; removes older libtool m4 fragments. Changes
Sequence Diagram(s)sequenceDiagram
participant Top as CMakeLists.txt
participant Module as cmake/libbacktrace.cmake
participant SystemLib as System libbacktrace
participant Vendored as Vendored build (ExternalProject/Imported)
participant MSVC as MSVC CMake build
Top->>Module: include(cmake/libbacktrace.cmake)
Module->>SystemLib: try find include & library (FLB_PREFER_SYSTEM_LIB_BACKTRACE)
alt system lib found
SystemLib-->>Module: provide include dirs & libs
Module-->>Top: set LIBBACKTRACE_LIBRARIES, FLB_HAVE_LIBBACKTRACE
else system lib not found
Module->>Vendored: choose fallback build path
alt MSVC environment
Module->>MSVC: add_sources / add_library(backtrace STATIC)
MSVC-->>Module: libbacktrace target available
else Unix-like
Module->>Vendored: ExternalProject_Add(configure/make/install)
Vendored-->>Module: create imported libbacktrace target
end
Module-->>Top: set LIBBACKTRACE_LIBRARIES, LIBBACKTRACE_FOUND=ON, FLB_HAVE_LIBBACKTRACE
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7eb14cb1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if(FLB_WINDOWS_DEFAULTS) | ||
| message(STATUS "Overriding setttings with windows-setup.cmake") | ||
| set(FLB_REGEX Yes) | ||
| set(FLB_BACKTRACE No) | ||
| set(FLB_BACKTRACE Yes) | ||
| set(FLB_LUAJIT Yes) |
There was a problem hiding this comment.
This file explicitly targets MSVC builds, but switching FLB_BACKTRACE to Yes here forces all Windows builds to invoke the libbacktrace ExternalProject that runs a configure script and $(MAKE) (see CMakeLists.txt around the libbacktrace ExternalProject). That autoconf/make workflow assumes a POSIX shell and GNU make, which the documented Windows/MSVC toolchain does not include, so the default Windows build path will now fail at the configure step unless users add MSYS/MinGW. Consider keeping this off for MSVC defaults or adding a Windows-specific libbacktrace build path.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/libbacktrace-b9e4006/pecoff.c (1)
845-849: Pass the calculated slide to coff_initialize_syminfo, not the image base.
coff_initialize_syminforeceivesimage_base(the loaded PE base address) but uses it as a relocation offset vialibbacktrace_add_base()at line 497. The DWARF path correctly computes the slide asmodule_handle - image_base(line 917) before passing tobacktrace_dwarf_add. The COFF path should do the same: passbase_address(the calculated slide) tocoff_initialize_syminfoinstead ofimage_base, ensuring symbol addresses are adjusted correctly for relocated images (ASLR).
🤖 Fix all issues with AI agents
In `@lib/libbacktrace-b9e4006/config/libtool.m4`:
- Around line 1098-1113: The CXX branch sets _lt_install_name but
archive_expsym_cmds still hardcodes \$rpath/\$soname, causing mismatched
install_name for symbol-export builds; update the archive_expsym_cmds assignment
in the m4_if([CXX], ...) block to use ${_lt_install_name} instead of
\$rpath/\$soname (i.e., replace the hardcoded install_name in the
archive_expsym_cmds string so it references _lt_install_name), and verify any
analogous appearance in module_expsym_cmds if present.
- Around line 3281-3335: The loop over candidate nm commands in LT_PATH_NM is
wrong because lt_nm_to_check is quoted, treating the entire string (prefixed nm
plus fallback) as one token and preventing fallback discovery; change the
iteration to split lt_nm_to_check into separate word tokens (e.g., use for
lt_tmp_nm in $lt_nm_to_check; do) so each candidate is tried individually while
still keeping the existing logic that strips user-supplied NM options
(tmp_nm_path / tmp_nm_to_nm) and the BSD/MS-compat checks; update references in
the LT_PATH_NM macro (lt_nm_to_check, lt_tmp_nm, tmp_nm_path, tmp_nm_to_nm)
accordingly so the fallback unprefixed nm is actually tested.
In `@lib/libbacktrace-b9e4006/dwarf.c`:
- Around line 1791-1798: The call to resolve_unit_addrs_overlap_walk in this
block ignores its return value and proceeds to free the old vector and assign
new_vec even on failure; update the code to check the return of
resolve_unit_addrs_overlap_walk (the same function used at line 1690), and if it
returns 0, free new_vec with backtrace_vector_free(&new_vec.vec, error_callback,
data) (to avoid leaking its partial contents) and return 0 without touching the
original addrs_vec; only on success free the old addrs_vec and assign *addrs_vec
= new_vec and return 1. Ensure you reference resolve_unit_addrs_overlap_walk,
new_vec, addrs_vec, and backtrace_vector_free in the fix.
In `@lib/libbacktrace-b9e4006/pecoff.c`:
- Around line 953-965: The code incorrectly assigns error_callback from
dll_context->data (casting a data pointer to a function pointer), causing
crashes/warnings; change the assignment in dll_notification so
backtrace_error_callback error_callback is taken from
dll_context->error_callback (with appropriate cast if required) and keep
dll_context->data assigned to the local data variable; update the line
referencing error_callback accordingly so it uses the correct function pointer
member of struct dll_notification_context.
In `@lib/libbacktrace-b9e4006/zstdtest.c`:
- Around line 193-211: The code treats malloc(0) as an allocation failure for
the empty test sample; change the allocation logic in the loop that sets
uncompressed_len and allocates uncompressed (around variables uncompressed_len
and uncompressed) so that when uncompressed_len == 0 you either allocate a
non-zero buffer (e.g., allocate 1 byte) or explicitly allow a NULL result and
handle it as a valid empty buffer before calling the decompressor; update the
error branch to only treat real allocation failures as errors and ensure
tests[i].name and failures handling remain unchanged.
🧹 Nitpick comments (2)
lib/libbacktrace-b9e4006/zstdtest.c (1)
421-502: Consider routing early exits through a cleanup path.Several
returnbranches after allocations skip freeingorig_buf,compressed_buf, anduncompressed_buf. A cleanup label (or reusingfail:) would keep the test leak-free even when timing calls fail.lib/libbacktrace-b9e4006/Makefile.am (1)
501-539: Avoid duplicatinglibbacktrace_testing_ldflagsin alloc LDFLAGS.
ctestg_LDFLAGS/ctesta_LDFLAGS/ctestzstd_LDFLAGSalready include the testing flags, so the alloc variants don’t need to re-append them.♻️ Suggested cleanup
-ctestg_alloc_LDFLAGS = $(ctestg_LDFLAGS) $(libbacktrace_testing_ldflags) +ctestg_alloc_LDFLAGS = $(ctestg_LDFLAGS) @@ -ctesta_alloc_LDFLAGS = $(ctesta_LDFLAGS) $(libbacktrace_testing_ldflags) +ctesta_alloc_LDFLAGS = $(ctesta_LDFLAGS) @@ -ctestzstd_alloc_LDFLAGS = $(ctestzstd_LDFLAGS) $(libbacktrace_testing_ldflags) +ctestzstd_alloc_LDFLAGS = $(ctestzstd_LDFLAGS)
| _lt_install_name='\$rpath/\$soname' | ||
| if test "x$enable_darwin_at_rpath" = "xyes"; then | ||
| _lt_install_name='@rpath/\$soname' | ||
| fi | ||
| _LT_TAGVAR(archive_cmds, $1)="\$CC -dynamiclib \$allow_undefined_flag -o \$lib \$libobjs \$deplibs \$compiler_flags -install_name ${_lt_install_name} \$verstring ${_lt_dsymutil}" | ||
| _LT_TAGVAR(module_cmds, $1)="\$CC \$allow_undefined_flag -o \$lib -bundle \$libobjs \$deplibs \$compiler_flags${_lt_dsymutil}" | ||
| _LT_TAGVAR(archive_expsym_cmds, $1)="sed 's,^,_,' < \$export_symbols > \$output_objdir/\${libname}-symbols.expsym~\$CC -dynamiclib \$allow_undefined_flag -o \$lib \$libobjs \$deplibs \$compiler_flags -install_name \$rpath/\$soname \$verstring ${_lt_dar_single_mod}${_lt_dar_export_syms}${_lt_dsymutil}" | ||
| _LT_TAGVAR(archive_expsym_cmds, $1)="sed 's,^,_,' < \$export_symbols > \$output_objdir/\${libname}-symbols.expsym~\$CC -dynamiclib \$allow_undefined_flag -o \$lib \$libobjs \$deplibs \$compiler_flags -install_name ${_lt_install_name} \$verstring ${_lt_dar_export_syms}${_lt_dsymutil}" | ||
| _LT_TAGVAR(module_expsym_cmds, $1)="sed -e 's,^,_,' < \$export_symbols > \$output_objdir/\${libname}-symbols.expsym~\$CC \$allow_undefined_flag -o \$lib -bundle \$libobjs \$deplibs \$compiler_flags${_lt_dar_export_syms}${_lt_dsymutil}" | ||
| m4_if([$1], [CXX], | ||
| [ if test "$lt_cv_apple_cc_single_mod" != "yes"; then | ||
| _LT_TAGVAR(archive_cmds, $1)="\$CC -r -keep_private_externs -nostdlib -o \${lib}-master.o \$libobjs~\$CC -dynamiclib \$allow_undefined_flag -o \$lib \${lib}-master.o \$deplibs \$compiler_flags -install_name \$rpath/\$soname \$verstring${_lt_dsymutil}" | ||
| _lt_install_name='\$rpath/\$soname' | ||
| if test "x$enable_darwin_at_rpath" = "xyes"; then | ||
| _lt_install_name='@rpath/\$soname' | ||
| fi | ||
| _LT_TAGVAR(archive_cmds, $1)="\$CC -r -keep_private_externs -nostdlib -o \${lib}-master.o \$libobjs~\$CC -dynamiclib \$allow_undefined_flag -o \$lib \${lib}-master.o \$deplibs \$compiler_flags -install_name ${_lt_install_name} \$verstring${_lt_dsymutil}" | ||
| _LT_TAGVAR(archive_expsym_cmds, $1)="sed 's,^,_,' < \$export_symbols > \$output_objdir/\${libname}-symbols.expsym~\$CC -r -keep_private_externs -nostdlib -o \${lib}-master.o \$libobjs~\$CC -dynamiclib \$allow_undefined_flag -o \$lib \${lib}-master.o \$deplibs \$compiler_flags -install_name \$rpath/\$soname \$verstring${_lt_dar_export_syms}${_lt_dsymutil}" |
There was a problem hiding this comment.
Use _lt_install_name in CXX export-symbol builds too.
When enable_darwin_at_rpath=yes, archive_cmds uses @rpath/..., but archive_expsym_cmds still hardcodes $rpath/$soname, which can produce a mismatched install_name for symbol-export builds.
🔧 Proposed fix
- _LT_TAGVAR(archive_expsym_cmds, $1)="sed 's,^,_,' < \$export_symbols > \$output_objdir/\${libname}-symbols.expsym~\$CC -r -keep_private_externs -nostdlib -o \${lib}-master.o \$libobjs~\$CC -dynamiclib \$allow_undefined_flag -o \$lib \${lib}-master.o \$deplibs \$compiler_flags -install_name \$rpath/\$soname \$verstring${_lt_dar_export_syms}${_lt_dsymutil}"
+ _LT_TAGVAR(archive_expsym_cmds, $1)="sed 's,^,_,' < \$export_symbols > \$output_objdir/\${libname}-symbols.expsym~\$CC -r -keep_private_externs -nostdlib -o \${lib}-master.o \$libobjs~\$CC -dynamiclib \$allow_undefined_flag -o \$lib \${lib}-master.o \$deplibs \$compiler_flags -install_name ${_lt_install_name} \$verstring${_lt_dar_export_syms}${_lt_dsymutil}"🤖 Prompt for AI Agents
In `@lib/libbacktrace-b9e4006/config/libtool.m4` around lines 1098 - 1113, The CXX
branch sets _lt_install_name but archive_expsym_cmds still hardcodes
\$rpath/\$soname, causing mismatched install_name for symbol-export builds;
update the archive_expsym_cmds assignment in the m4_if([CXX], ...) block to use
${_lt_install_name} instead of \$rpath/\$soname (i.e., replace the hardcoded
install_name in the archive_expsym_cmds string so it references
_lt_install_name), and verify any analogous appearance in module_expsym_cmds if
present.
| # find the pathname to a BSD- or MS-compatible name lister, and any flags | ||
| # needed to make it compatible | ||
| AC_DEFUN([LT_PATH_NM], | ||
| [AC_REQUIRE([AC_PROG_CC])dnl | ||
| AC_CACHE_CHECK([for BSD- or MS-compatible name lister (nm)], lt_cv_path_NM, | ||
| [if test -n "$NM"; then | ||
| # Let the user override the test. | ||
| lt_cv_path_NM="$NM" | ||
| else | ||
| lt_nm_to_check="${ac_tool_prefix}nm" | ||
| if test -n "$ac_tool_prefix" && test "$build" = "$host"; then | ||
| lt_nm_to_check="$lt_nm_to_check nm" | ||
| fi | ||
| for lt_tmp_nm in $lt_nm_to_check; do | ||
| lt_save_ifs="$IFS"; IFS=$PATH_SEPARATOR | ||
| for ac_dir in $PATH /usr/ccs/bin/elf /usr/ccs/bin /usr/ucb /bin; do | ||
| IFS="$lt_save_ifs" | ||
| test -z "$ac_dir" && ac_dir=. | ||
| tmp_nm="$ac_dir/$lt_tmp_nm" | ||
| if test -f "$tmp_nm" || test -f "$tmp_nm$ac_exeext" ; then | ||
| # Check to see if the nm accepts a BSD-compat flag. | ||
| # Adding the `sed 1q' prevents false positives on HP-UX, which says: | ||
| # nm: unknown option "B" ignored | ||
| # Tru64's nm complains that /dev/null is an invalid object file | ||
| case `"$tmp_nm" -B /dev/null 2>&1 | sed '1q'` in | ||
| */dev/null* | *'Invalid file or object type'*) | ||
| lt_cv_path_NM="$tmp_nm -B" | ||
| break | ||
| ;; | ||
| *) | ||
| case `"$tmp_nm" -p /dev/null 2>&1 | sed '1q'` in | ||
| */dev/null*) | ||
| lt_cv_path_NM="$tmp_nm -p" | ||
| break | ||
| ;; | ||
| *) | ||
| lt_cv_path_NM=${lt_cv_path_NM="$tmp_nm"} # keep the first match, but | ||
| continue # so that we can try to find one that supports BSD flags | ||
| ;; | ||
| esac | ||
| ;; | ||
| esac | ||
| fi | ||
| done | ||
| IFS="$lt_save_ifs" | ||
| done | ||
| : ${lt_cv_path_NM=no} | ||
| fi]) | ||
| # Let the user override the nm to test. | ||
| lt_nm_to_check="$NM" | ||
| else | ||
| lt_nm_to_check="${ac_tool_prefix}nm" | ||
| if test -n "$ac_tool_prefix" && test "$build" = "$host"; then | ||
| lt_nm_to_check="$lt_nm_to_check nm" | ||
| fi | ||
| fi | ||
| for lt_tmp_nm in "$lt_nm_to_check"; do | ||
| lt_save_ifs="$IFS"; IFS=$PATH_SEPARATOR | ||
| for ac_dir in $PATH /usr/ccs/bin/elf /usr/ccs/bin /usr/ucb /bin; do | ||
| IFS="$lt_save_ifs" | ||
| test -z "$ac_dir" && ac_dir=. | ||
| # Strip out any user-provided options from the nm to test twice, | ||
| # the first time to test to see if nm (rather than its options) has | ||
| # an explicit path, the second time to yield a file which can be | ||
| # nm'ed itself. | ||
| tmp_nm_path="`$ECHO "$lt_tmp_nm" | sed 's, -.*$,,'`" | ||
| case "$tmp_nm_path" in | ||
| */*|*\\*) tmp_nm="$lt_tmp_nm";; | ||
| *) tmp_nm="$ac_dir/$lt_tmp_nm";; | ||
| esac | ||
| tmp_nm_to_nm="`$ECHO "$tmp_nm" | sed 's, -.*$,,'`" | ||
| if test -f "$tmp_nm_to_nm" || test -f "$tmp_nm_to_nm$ac_exeext" ; then | ||
| # Check to see if the nm accepts a BSD-compat flag. | ||
| # Adding the `sed 1q' prevents false positives on HP-UX, which says: | ||
| # nm: unknown option "B" ignored | ||
| case `"$tmp_nm" -B "$tmp_nm_to_nm" 2>&1 | grep -v '^ *$' | sed '1q'` in | ||
| *$tmp_nm*) lt_cv_path_NM="$tmp_nm -B" | ||
| break | ||
| ;; | ||
| *) | ||
| case `"$tmp_nm" -p "$tmp_nm_to_nm" 2>&1 | grep -v '^ *$' | sed '1q'` in | ||
| *$tmp_nm*) | ||
| lt_cv_path_NM="$tmp_nm -p" | ||
| break | ||
| ;; | ||
| *) | ||
| lt_cv_path_NM=${lt_cv_path_NM="$tmp_nm"} # keep the first match, but | ||
| continue # so that we can try to find one that supports BSD flags | ||
| ;; | ||
| esac | ||
| ;; | ||
| esac | ||
| fi | ||
| done | ||
| IFS="$lt_save_ifs" | ||
| done | ||
| : ${lt_cv_path_NM=no}]) |
There was a problem hiding this comment.
Fix nm candidate iteration when ac_tool_prefix fallback is needed.
lt_nm_to_check can contain both the prefixed nm and the unprefixed fallback, but quoting the loop variable turns that into a single (invalid) command string, preventing fallback discovery. Split candidates while still preserving user-supplied NM options.
🔧 Proposed fix
- else
- lt_nm_to_check="${ac_tool_prefix}nm"
- if test -n "$ac_tool_prefix" && test "$build" = "$host"; then
- lt_nm_to_check="$lt_nm_to_check nm"
- fi
- fi
- for lt_tmp_nm in "$lt_nm_to_check"; do
+ else
+ lt_nm_to_check="${ac_tool_prefix}nm"
+ if test -n "$ac_tool_prefix" && test "$build" = "$host"; then
+ lt_nm_to_check="$lt_nm_to_check
+nm"
+ fi
+ fi
+ lt_nm_save_ifs="$IFS"; IFS='
+'
+ for lt_tmp_nm in $lt_nm_to_check; do
lt_save_ifs="$IFS"; IFS=$PATH_SEPARATOR
...
IFS="$lt_save_ifs"
done
+ IFS="$lt_nm_save_ifs"🤖 Prompt for AI Agents
In `@lib/libbacktrace-b9e4006/config/libtool.m4` around lines 3281 - 3335, The
loop over candidate nm commands in LT_PATH_NM is wrong because lt_nm_to_check is
quoted, treating the entire string (prefixed nm plus fallback) as one token and
preventing fallback discovery; change the iteration to split lt_nm_to_check into
separate word tokens (e.g., use for lt_tmp_nm in $lt_nm_to_check; do) so each
candidate is tried individually while still keeping the existing logic that
strips user-supplied NM options (tmp_nm_path / tmp_nm_to_nm) and the
BSD/MS-compat checks; update references in the LT_PATH_NM macro (lt_nm_to_check,
lt_tmp_nm, tmp_nm_path, tmp_nm_to_nm) accordingly so the fallback unprefixed nm
is actually tested.
| from = 0; | ||
| to = 0; | ||
| resolve_unit_addrs_overlap_walk (state, &from, &to, NULL, addrs_vec, | ||
| error_callback, data, &new_vec); | ||
| backtrace_vector_free (state, &addrs_vec->vec, error_callback, data); | ||
| *addrs_vec = new_vec; | ||
|
|
||
| return 1; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
libbacktrace resolve_unit_addrs_overlap_walk return value
💡 Result:
In libbacktrace (dwarf.c), resolve_unit_addrs_overlap_walk(...) returns an int used as a boolean success flag:
- Returns
0only on failure—specifically whenbacktrace_vector_grow(...)fails (allocation/grow failure). (gcc.gnu.org) - Returns
1on success, including the “early-exit” case used to unwind recursion when anenclosingrange can’t cover any further entries (it updates*pfrom/*ptoand returns to the caller). (gcc.gnu.org)
Citations:
- 1: https://gcc.gnu.org/pipermail/gcc-patches/2024-October/665890.html?utm_source=openai
- 2: https://gcc.gnu.org/pipermail/gcc-patches/2024-October/665890.html?utm_source=openai
🏁 Script executed:
# First, let's verify the code structure and check the actual implementation
fd -t f "dwarf.c" lib/libbacktrace*Repository: fluent/fluent-bit
Length of output: 93
🏁 Script executed:
# Read the area around lines 1791-1798 and the function implementation
wc -l lib/libbacktrace-b9e4006/dwarf.cRepository: fluent/fluent-bit
Length of output: 98
🏁 Script executed:
# Read the resolve_unit_addrs_overlap function (lines 1747-1799)
sed -n '1747,1810p' lib/libbacktrace-b9e4006/dwarf.cRepository: fluent/fluent-bit
Length of output: 1691
🏁 Script executed:
# Also check the resolve_unit_addrs_overlap_walk function to see when it returns 0
sed -n '1616,1720p' lib/libbacktrace-b9e4006/dwarf.cRepository: fluent/fluent-bit
Length of output: 2708
Unchecked return value from resolve_unit_addrs_overlap_walk at lines 1793–1794.
The function call ignores its return value. The resolve_unit_addrs_overlap_walk function returns 0 on failure (when internal backtrace_vector_grow calls fail), but this failure is silently discarded. The caller then proceeds to free the old vector and use the potentially incomplete new_vec, returning 1 (success) regardless.
This is inconsistent with the recursive call to the same function at line 1690–1692, which properly checks the return value. The fix should check for failure and clean up:
🔧 Proposed fix
from = 0;
to = 0;
- resolve_unit_addrs_overlap_walk (state, &from, &to, NULL, addrs_vec,
- error_callback, data, &new_vec);
+ if (!resolve_unit_addrs_overlap_walk (state, &from, &to, NULL, addrs_vec,
+ error_callback, data, &new_vec))
+ {
+ backtrace_vector_free (state, &new_vec.vec, error_callback, data);
+ return 0;
+ }
backtrace_vector_free (state, &addrs_vec->vec, error_callback, data);
*addrs_vec = new_vec;
return 1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from = 0; | |
| to = 0; | |
| resolve_unit_addrs_overlap_walk (state, &from, &to, NULL, addrs_vec, | |
| error_callback, data, &new_vec); | |
| backtrace_vector_free (state, &addrs_vec->vec, error_callback, data); | |
| *addrs_vec = new_vec; | |
| return 1; | |
| from = 0; | |
| to = 0; | |
| if (!resolve_unit_addrs_overlap_walk (state, &from, &to, NULL, addrs_vec, | |
| error_callback, data, &new_vec)) | |
| { | |
| backtrace_vector_free (state, &new_vec.vec, error_callback, data); | |
| return 0; | |
| } | |
| backtrace_vector_free (state, &addrs_vec->vec, error_callback, data); | |
| *addrs_vec = new_vec; | |
| return 1; |
🤖 Prompt for AI Agents
In `@lib/libbacktrace-b9e4006/dwarf.c` around lines 1791 - 1798, The call to
resolve_unit_addrs_overlap_walk in this block ignores its return value and
proceeds to free the old vector and assign new_vec even on failure; update the
code to check the return of resolve_unit_addrs_overlap_walk (the same function
used at line 1690), and if it returns 0, free new_vec with
backtrace_vector_free(&new_vec.vec, error_callback, data) (to avoid leaking its
partial contents) and return 0 without touching the original addrs_vec; only on
success free the old addrs_vec and assign *addrs_vec = new_vec and return 1.
Ensure you reference resolve_unit_addrs_overlap_walk, new_vec, addrs_vec, and
backtrace_vector_free in the fix.
| static VOID CALLBACK | ||
| dll_notification (ULONG reason, | ||
| struct dll_notification_data *notification_data, | ||
| PVOID context) | ||
| { | ||
| char module_name[MAX_PATH]; | ||
| int descriptor; | ||
| struct dll_notification_context* dll_context = | ||
| (struct dll_notification_context*) context; | ||
| struct backtrace_state *state = dll_context->state; | ||
| void *data = dll_context->data; | ||
| backtrace_error_callback error_callback = dll_context->data; | ||
| fileline fileline; |
There was a problem hiding this comment.
Fix DLL notification error_callback wiring.
error_callback is pulled from dll_context->data instead of dll_context->error_callback, which turns a data pointer into a function pointer and can crash or fail builds with warnings.
🐛 Proposed fix
- backtrace_error_callback error_callback = dll_context->data;
+ backtrace_error_callback error_callback = dll_context->error_callback;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static VOID CALLBACK | |
| dll_notification (ULONG reason, | |
| struct dll_notification_data *notification_data, | |
| PVOID context) | |
| { | |
| char module_name[MAX_PATH]; | |
| int descriptor; | |
| struct dll_notification_context* dll_context = | |
| (struct dll_notification_context*) context; | |
| struct backtrace_state *state = dll_context->state; | |
| void *data = dll_context->data; | |
| backtrace_error_callback error_callback = dll_context->data; | |
| fileline fileline; | |
| static VOID CALLBACK | |
| dll_notification (ULONG reason, | |
| struct dll_notification_data *notification_data, | |
| PVOID context) | |
| { | |
| char module_name[MAX_PATH]; | |
| int descriptor; | |
| struct dll_notification_context* dll_context = | |
| (struct dll_notification_context*) context; | |
| struct backtrace_state *state = dll_context->state; | |
| void *data = dll_context->data; | |
| backtrace_error_callback error_callback = dll_context->error_callback; | |
| fileline fileline; |
🤖 Prompt for AI Agents
In `@lib/libbacktrace-b9e4006/pecoff.c` around lines 953 - 965, The code
incorrectly assigns error_callback from dll_context->data (casting a data
pointer to a function pointer), causing crashes/warnings; change the assignment
in dll_notification so backtrace_error_callback error_callback is taken from
dll_context->error_callback (with appropriate cast if required) and keep
dll_context->data assigned to the local data variable; update the line
referencing error_callback accordingly so it uses the correct function pointer
member of struct dll_notification_context.
| size_t i; | ||
|
|
||
| for (i = 0; i < sizeof tests / sizeof tests[0]; ++i) | ||
| { | ||
| unsigned char *uncompressed; | ||
| size_t uncompressed_len; | ||
|
|
||
| uncompressed_len = tests[i].uncompressed_len; | ||
| if (uncompressed_len == 0) | ||
| uncompressed_len = strlen (tests[i].uncompressed); | ||
|
|
||
| uncompressed = (unsigned char *) malloc (uncompressed_len); | ||
| if (uncompressed == NULL) | ||
| { | ||
| perror ("malloc"); | ||
| fprintf (stderr, "test %s: uncompress failed\n", tests[i].name); | ||
| ++failures; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Avoid treating malloc(0) as a hard failure for the empty sample.
uncompressed_len is 0 for the “empty” case, and malloc(0) may legally return NULL, which is currently treated as an error and can pass a NULL buffer to the decompressor.
🔧 Suggested fix
- unsigned char *uncompressed;
- size_t uncompressed_len;
+ unsigned char *uncompressed;
+ size_t uncompressed_len;
+ size_t alloc_len;
@@
- uncompressed = (unsigned char *) malloc (uncompressed_len);
+ alloc_len = uncompressed_len == 0 ? 1 : uncompressed_len;
+ uncompressed = (unsigned char *) malloc (alloc_len);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| size_t i; | |
| for (i = 0; i < sizeof tests / sizeof tests[0]; ++i) | |
| { | |
| unsigned char *uncompressed; | |
| size_t uncompressed_len; | |
| uncompressed_len = tests[i].uncompressed_len; | |
| if (uncompressed_len == 0) | |
| uncompressed_len = strlen (tests[i].uncompressed); | |
| uncompressed = (unsigned char *) malloc (uncompressed_len); | |
| if (uncompressed == NULL) | |
| { | |
| perror ("malloc"); | |
| fprintf (stderr, "test %s: uncompress failed\n", tests[i].name); | |
| ++failures; | |
| continue; | |
| } | |
| size_t i; | |
| for (i = 0; i < sizeof tests / sizeof tests[0]; ++i) | |
| { | |
| unsigned char *uncompressed; | |
| size_t uncompressed_len; | |
| size_t alloc_len; | |
| uncompressed_len = tests[i].uncompressed_len; | |
| if (uncompressed_len == 0) | |
| uncompressed_len = strlen (tests[i].uncompressed); | |
| alloc_len = uncompressed_len == 0 ? 1 : uncompressed_len; | |
| uncompressed = (unsigned char *) malloc (alloc_len); | |
| if (uncompressed == NULL) | |
| { | |
| perror ("malloc"); | |
| fprintf (stderr, "test %s: uncompress failed\n", tests[i].name); | |
| +failures; | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In `@lib/libbacktrace-b9e4006/zstdtest.c` around lines 193 - 211, The code treats
malloc(0) as an allocation failure for the empty test sample; change the
allocation logic in the loop that sets uncompressed_len and allocates
uncompressed (around variables uncompressed_len and uncompressed) so that when
uncompressed_len == 0 you either allocate a non-zero buffer (e.g., allocate 1
byte) or explicitly allow a NULL result and handle it as a valid empty buffer
before calling the decompressor; update the error branch to only treat real
allocation failures as errors and ensure tests[i].name and failures handling
remain unchanged.
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@cmake/libbacktrace.cmake`:
- Around line 13-19: The call to link_directories(${LIBBACKTRACE_LIBRARY_DIRS})
is ineffective because LIBBACKTRACE_LIBRARY_DIRS is never set; update the
libbacktrace handling to either remove the link_directories call entirely (since
LIBBACKTRACE_LIBRARIES contains the full library path) or derive the directory
from LIBBACKTRACE_LIBRARIES (e.g., compute its parent directory and assign it to
LIBBACKTRACE_LIBRARY_DIRS) before calling link_directories; adjust the branch
that checks LIBBACKTRACE_INCLUDE_DIRS AND LIBBACKTRACE_LIBRARIES and ensure
FLB_DEFINITION(FLB_HAVE_LIBBACKTRACE) remains set after fixing.
- Around line 96-118: DEPS_C_COMPILER is built to include -isysroot on macOS but
never passed to ExternalProject_Add's CONFIGURE_COMMAND, so configure can't see
the right headers; update the backtrace ExternalProject_Add CONFIGURE_COMMAND to
export/use DEPS_C_COMPILER as the C compiler (e.g., prefix the configure
invocation with CC="${DEPS_C_COMPILER}" or use env CC="${DEPS_C_COMPILER}") so
the configure script invoked via LIBBACKTRACE_SOURCE_DIR/configure receives the
correct compiler/flags; ensure you reference DEPS_C_COMPILER in the
CONFIGURE_COMMAND line inside ExternalProject_Add.
🧹 Nitpick comments (1)
cmake/libbacktrace.cmake (1)
67-78: Consider adding compression library definitions for DWARF debug info support.The libbacktrace library supports reading compressed debug sections (zlib/zstd). The current compile definitions don't include
HAVE_ZLIBorHAVE_ZSTD. If Fluent Bit's Windows builds include compressed DWARF debug info, stacktraces may fail to resolve symbols.This may be intentional if Windows builds don't use compressed debug info, but worth confirming.
💡 Optional: Add compression support if needed
target_compile_definitions(libbacktrace PRIVATE HAVE_WINDOWS_H=1 HAVE_DECL__PGMPTR=1 HAVE_STDLIB_H=1 HAVE_STRING_H=1 HAVE_STDINT_H=1 STDC_HEADERS=1 BACKTRACE_ELF_SIZE=0 BACKTRACE_XCOFF_SIZE=0 + # Uncomment if zlib/zstd support is needed for compressed debug info + # HAVE_ZLIB=1 + # HAVE_ZSTD=1 PACKAGE_NAME="libbacktrace" PACKAGE_VERSION="1.0" )
| if(LIBBACKTRACE_INCLUDE_DIRS AND LIBBACKTRACE_LIBRARIES) | ||
| message(STATUS "libbacktrace: using system library (${LIBBACKTRACE_LIBRARIES})") | ||
| include_directories(${LIBBACKTRACE_INCLUDE_DIRS}) | ||
| link_directories(${LIBBACKTRACE_LIBRARY_DIRS}) | ||
| set(LIBBACKTRACE_FOUND TRUE) | ||
| FLB_DEFINITION(FLB_HAVE_LIBBACKTRACE) | ||
| endif() |
There was a problem hiding this comment.
LIBBACKTRACE_LIBRARY_DIRS is undefined; link_directories() call is ineffective.
find_library() sets LIBBACKTRACE_LIBRARIES to the full path of the library, but does not set LIBBACKTRACE_LIBRARY_DIRS. The link_directories() call on line 16 will use an empty/undefined variable.
🔧 Suggested fix
Either remove the link_directories() call (since LIBBACKTRACE_LIBRARIES contains the full path), or extract the directory:
if(LIBBACKTRACE_INCLUDE_DIRS AND LIBBACKTRACE_LIBRARIES)
message(STATUS "libbacktrace: using system library (${LIBBACKTRACE_LIBRARIES})")
include_directories(${LIBBACKTRACE_INCLUDE_DIRS})
- link_directories(${LIBBACKTRACE_LIBRARY_DIRS})
+ # link_directories not needed - LIBBACKTRACE_LIBRARIES contains full path
set(LIBBACKTRACE_FOUND TRUE)
FLB_DEFINITION(FLB_HAVE_LIBBACKTRACE)
endif()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if(LIBBACKTRACE_INCLUDE_DIRS AND LIBBACKTRACE_LIBRARIES) | |
| message(STATUS "libbacktrace: using system library (${LIBBACKTRACE_LIBRARIES})") | |
| include_directories(${LIBBACKTRACE_INCLUDE_DIRS}) | |
| link_directories(${LIBBACKTRACE_LIBRARY_DIRS}) | |
| set(LIBBACKTRACE_FOUND TRUE) | |
| FLB_DEFINITION(FLB_HAVE_LIBBACKTRACE) | |
| endif() | |
| if(LIBBACKTRACE_INCLUDE_DIRS AND LIBBACKTRACE_LIBRARIES) | |
| message(STATUS "libbacktrace: using system library (${LIBBACKTRACE_LIBRARIES})") | |
| include_directories(${LIBBACKTRACE_INCLUDE_DIRS}) | |
| # link_directories not needed - LIBBACKTRACE_LIBRARIES contains full path | |
| set(LIBBACKTRACE_FOUND TRUE) | |
| FLB_DEFINITION(FLB_HAVE_LIBBACKTRACE) | |
| endif() |
🤖 Prompt for AI Agents
In `@cmake/libbacktrace.cmake` around lines 13 - 19, The call to
link_directories(${LIBBACKTRACE_LIBRARY_DIRS}) is ineffective because
LIBBACKTRACE_LIBRARY_DIRS is never set; update the libbacktrace handling to
either remove the link_directories call entirely (since LIBBACKTRACE_LIBRARIES
contains the full library path) or derive the directory from
LIBBACKTRACE_LIBRARIES (e.g., compute its parent directory and assign it to
LIBBACKTRACE_LIBRARY_DIRS) before calling link_directories; adjust the branch
that checks LIBBACKTRACE_INCLUDE_DIRS AND LIBBACKTRACE_LIBRARIES and ensure
FLB_DEFINITION(FLB_HAVE_LIBBACKTRACE) remains set after fixing.
| if(CMAKE_OSX_SYSROOT) | ||
| # From macOS Mojave, /usr/include does not store C SDK headers. | ||
| # For libbacktrace building on macOS, we have to tell C headers where they are located. | ||
| set(DEPS_C_COMPILER "${CMAKE_C_COMPILER} -isysroot ${CMAKE_OSX_SYSROOT}") | ||
| else() | ||
| set(DEPS_C_COMPILER "${CMAKE_C_COMPILER}") | ||
| endif() | ||
|
|
||
| # Set up build paths | ||
| set(FLB_LIBBACKTRACE_PATH "${CMAKE_CURRENT_BINARY_DIR}/backtrace-prefix/lib/libbacktrace.a") | ||
|
|
||
| # Build using ExternalProject with autoconf/configure | ||
| ExternalProject_Add(backtrace | ||
| SOURCE_DIR ${LIBBACKTRACE_SOURCE_DIR} | ||
| CONFIGURE_COMMAND ${LIBBACKTRACE_SOURCE_DIR}/configure | ||
| ${AUTOCONF_HOST_OPT} | ||
| --prefix=<INSTALL_DIR> | ||
| --enable-shared=no | ||
| --enable-static=yes | ||
| BUILD_COMMAND ${EXTERNAL_BUILD_TOOL} | ||
| BUILD_BYPRODUCTS ${FLB_LIBBACKTRACE_PATH} | ||
| INSTALL_COMMAND ${EXTERNAL_BUILD_TOOL} DESTDIR= install | ||
| ) |
There was a problem hiding this comment.
DEPS_C_COMPILER is set but never passed to the configure command.
The macOS SDK path handling correctly constructs DEPS_C_COMPILER with -isysroot, but this variable is never passed to the ExternalProject_Add configure command. On macOS with a non-default SDK (e.g., when using Xcode command-line tools), the configure script won't find the correct C headers.
🐛 Suggested fix
# Build using ExternalProject with autoconf/configure
ExternalProject_Add(backtrace
SOURCE_DIR ${LIBBACKTRACE_SOURCE_DIR}
CONFIGURE_COMMAND ${LIBBACKTRACE_SOURCE_DIR}/configure
${AUTOCONF_HOST_OPT}
--prefix=<INSTALL_DIR>
--enable-shared=no
--enable-static=yes
+ "CC=${DEPS_C_COMPILER}"
BUILD_COMMAND ${EXTERNAL_BUILD_TOOL}
BUILD_BYPRODUCTS ${FLB_LIBBACKTRACE_PATH}
INSTALL_COMMAND ${EXTERNAL_BUILD_TOOL} DESTDIR= install
)🤖 Prompt for AI Agents
In `@cmake/libbacktrace.cmake` around lines 96 - 118, DEPS_C_COMPILER is built to
include -isysroot on macOS but never passed to ExternalProject_Add's
CONFIGURE_COMMAND, so configure can't see the right headers; update the
backtrace ExternalProject_Add CONFIGURE_COMMAND to export/use DEPS_C_COMPILER as
the C compiler (e.g., prefix the configure invocation with
CC="${DEPS_C_COMPILER}" or use env CC="${DEPS_C_COMPILER}") so the configure
script invoked via LIBBACKTRACE_SOURCE_DIR/configure receives the correct
compiler/flags; ensure you reference DEPS_C_COMPILER in the CONFIGURE_COMMAND
line inside ExternalProject_Add.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/call-windows-unit-tests.yaml (1)
58-70: Remove the hardcoded CMake path to ensure the specified version is used.The hardcoded
C:/Program Files/CMake/binin the PATH will resolve before the action-installed cmake, causing the pre-installed system cmake to be used instead of version3.31.6. Sincejwlawson/actions-setup-cmake@v2appends toGITHUB_PATH, it comes after the existing PATH. Remove the hardcoded CMake path or add a version verification step after setup.🔧 Suggested fix
env: - PATH: C:\ProgramData\Chocolatey\bin;c:/Program Files/Git/cmd;c:/Windows/system32;C:/Windows/System32/WindowsPowerShell/v1.0;$ENV:WIX/bin;C:/Program Files/CMake/bin;C:\vcpkg; + PATH: C:\ProgramData\Chocolatey\bin;c:/Program Files/Git/cmd;c:/Windows/system32;C:/Windows/System32/WindowsPowerShell/v1.0;$ENV:WIX/bin;C:\vcpkg; - name: Set up CMake uses: jwlawson/actions-setup-cmake@v2 with: cmake-version: '${{ matrix.config.cmake_version }}' + - name: Verify CMake version + run: cmake --version + shell: pwsh.github/workflows/call-build-windows.yaml (1)
101-113: Remove the hardcoded CMake path fromPATHfor clarity and to ensure the specified version is used.The hardcoded
C:/Program Files/CMake/binin thePATHenv variable creates ambiguity about which CMake binary is actually being used. Although thejwlawson/actions-setup-cmakeaction prepends its installed CMake directory toPATH(giving it precedence in most cases), this doesn't account for edge cases or make the intent clear. Removing the hardcoded path eliminates confusion and ensures that only the explicitly specifiedcmake_versionis available.Suggested fix
env: - PATH: C:\ProgramData\Chocolatey\bin;c:/Program Files/Git/cmd;c:/Windows/system32;C:/Windows/System32/WindowsPowerShell/v1.0;$ENV:WIX/bin;C:/Program Files/CMake/bin;C:\vcpkg; + PATH: C:\ProgramData\Chocolatey\bin;c:/Program Files/Git/cmd;c:/Windows/system32;C:/Windows/System32/WindowsPowerShell/v1.0;$ENV:WIX/bin;C:\vcpkg; steps: - name: Checkout repository uses: actions/checkout@v6 with: ref: ${{ inputs.ref }} - name: Set up CMake uses: jwlawson/actions-setup-cmake@v2 with: cmake-version: '${{ matrix.config.cmake_version }}' + + - name: Verify CMake version + run: cmake --version + shell: pwsh
edsiper
force-pushed
the
update-libbacktrace-20260128
branch
from
20f83ee to
effcad6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/call-windows-unit-tests.yaml:
- Around line 66-72: The cmake architecture is hardcoded to "x86_64" which
forces an emulated binary on ARM runners; update the logic that sets $cmakeArch
so it picks a native ARM64 build when running on an ARM64 runner (inspect
environment like $env:PROCESSOR_ARCHITECTURE or $env:RUNNER_ARCH and fall back
to x86_64 otherwise). Change the assignment for $cmakeArch (the variable used to
build $cmakeUrl) to conditionally use "arm64" for ARM64 hosts and "x86_64" for
others so the download URL (cmake-${cmakeVersion}-windows-${cmakeArch}.zip)
points at the correct native CMake release.
🧹 Nitpick comments (2)
.github/workflows/call-build-windows.yaml (2)
109-184: Consider extracting CMake setup to a reusable composite action.This ~75-line PowerShell script is duplicated verbatim in
call-windows-unit-tests.yaml. Extracting it to a composite action (e.g.,.github/actions/setup-cmake/action.yml) would:
- Eliminate duplication and reduce maintenance burden
- Allow single-point updates when CMake versions or download logic changes
- Keep workflow files focused on orchestration
Example composite action structure
Create
.github/actions/setup-cmake/action.yml:name: 'Setup CMake' description: 'Downloads and installs CMake from GitHub releases' inputs: version: description: 'CMake version to install' required: true runs: using: 'composite' steps: - name: Install CMake run: | # Move the PowerShell script here $cmakeVersion = "${{ inputs.version }}" # ... rest of the script shell: pwshThen in workflows:
- name: Set up CMake uses: ./.github/actions/setup-cmake with: version: ${{ matrix.config.cmake_version }}
152-159:-Encoding Byteis deprecated in PowerShell Core.The
pwshshell uses PowerShell Core where-Encoding Byteis deprecated. While it may still work for backward compatibility, the modern parameter is-AsByteStream.Suggested fix
- $header = Get-Content $cmakeZip -TotalCount 1 -Encoding Byte -ErrorAction SilentlyContinue + $header = Get-Content $cmakeZip -AsByteStream -TotalCount 2 -ErrorAction SilentlyContinue
| - name: Set up CMake | ||
| run: | | ||
| $cmakeVersion = "${{ matrix.config.cmake_version }}" | ||
| $cmakeArch = "x86_64" | ||
| $cmakeUrl = "https://github.com/Kitware/CMake/releases/download/v${cmakeVersion}/cmake-${cmakeVersion}-windows-${cmakeArch}.zip" | ||
| $cmakeZip = "$env:RUNNER_TEMP\cmake-${cmakeVersion}.zip" | ||
| $cmakeDir = "C:\Program Files\CMake" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
CMake ARM64 Windows release download available
💡 Result:
Yes—CMake provides native Windows ARM64 release downloads.
- The CMake download page lists Windows ARM64 Installer (
cmake-4.2.3-windows-arm64.msi) and Windows ARM64 ZIP (cmake-4.2.3-windows-arm64.zip) for the latest stable release. [1] - Native Windows-on-Arm support is available starting with CMake 3.24 (so you can pick an older ARM64 build if needed). [2]
Download page (choose the Windows ARM64 entries):
https://cmake.org/download/
Sources: [1], [2]
Hardcoded x86_64 architecture should use native ARM64 binary on ARM64 runner.
The ARM64 configuration uses os: windows-11-arm but $cmakeArch is hardcoded to "x86_64". While Windows on ARM can run x86_64 binaries through emulation, CMake provides native ARM64 Windows releases (available since CMake 3.24). Using the native ARM64 binary is preferable to emulation.
Suggested fix to detect and use native ARM64 builds
$cmakeVersion = "${{ matrix.config.cmake_version }}"
- $cmakeArch = "x86_64"
+ # Determine CMake architecture based on runner
+ if ($env:PROCESSOR_ARCHITECTURE -eq "ARM64") {
+ $cmakeArch = "arm64"
+ } else {
+ $cmakeArch = "x86_64"
+ }
$cmakeUrl = "https://github.com/Kitware/CMake/releases/download/v${cmakeVersion}/cmake-${cmakeVersion}-windows-${cmakeArch}.zip"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Set up CMake | |
| run: | | |
| $cmakeVersion = "${{ matrix.config.cmake_version }}" | |
| $cmakeArch = "x86_64" | |
| $cmakeUrl = "https://github.com/Kitware/CMake/releases/download/v${cmakeVersion}/cmake-${cmakeVersion}-windows-${cmakeArch}.zip" | |
| $cmakeZip = "$env:RUNNER_TEMP\cmake-${cmakeVersion}.zip" | |
| $cmakeDir = "C:\Program Files\CMake" | |
| - name: Set up CMake | |
| run: | | |
| $cmakeVersion = "${{ matrix.config.cmake_version }}" | |
| # Determine CMake architecture based on runner | |
| if ($env:PROCESSOR_ARCHITECTURE -eq "ARM64") { | |
| $cmakeArch = "arm64" | |
| } else { | |
| $cmakeArch = "x86_64" | |
| } | |
| $cmakeUrl = "https://github.com/Kitware/CMake/releases/download/v${cmakeVersion}/cmake-${cmakeVersion}-windows-${cmakeArch}.zip" | |
| $cmakeZip = "$env:RUNNER_TEMP\cmake-${cmakeVersion}.zip" | |
| $cmakeDir = "C:\Program Files\CMake" |
🤖 Prompt for AI Agents
In @.github/workflows/call-windows-unit-tests.yaml around lines 66 - 72, The
cmake architecture is hardcoded to "x86_64" which forces an emulated binary on
ARM runners; update the logic that sets $cmakeArch so it picks a native ARM64
build when running on an ARM64 runner (inspect environment like
$env:PROCESSOR_ARCHITECTURE or $env:RUNNER_ARCH and fall back to x86_64
otherwise). Change the assignment for $cmakeArch (the variable used to build
$cmakeUrl) to conditionally use "arm64" for ARM64 hosts and "x86_64" for others
so the download URL (cmake-${cmakeVersion}-windows-${cmakeArch}.zip) points at
the correct native CMake release.
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
edsiper
force-pushed
the
update-libbacktrace-20260128
branch
from
effcad6 to
c34e047
Compare
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
cmake/libbacktrace.cmakewith proper rules to build it on WindowsFluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.