Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/native/external/libunwind.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ elseif(CLR_CMAKE_TARGET_FREEBSD)
set(libunwind_la_SOURCES_arm_os arm/Gos-freebsd.c)
set(libunwind_la_SOURCES_arm_os_local arm/Los-freebsd.c)
set(libunwind_la_SOURCES_aarch64_os aarch64/Gos-freebsd.c)
set(libunwind_la_SOURCES_aarch64_os_local aarch64/Los-freebsd.c)
set(libunwind_la_SOURCES_aarch64_os_local aarch64/Los-freebsd.c aarch64/setcontext.S)
Copy link
Member

@am11 am11 Dec 5, 2024

Choose a reason for hiding this comment

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

We need both (only setcontext.S was missing)

Suggested change
set(libunwind_la_SOURCES_aarch64_os_local aarch64/Los-freebsd.c aarch64/setcontext.S)
set(libunwind_la_SOURCES_aarch64_os_local aarch64/Los-freebsd.c)
set(libunwind_aarch64_la_SOURCES_os aarch64/setcontext.S)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't what my change set(libunwind_la_SOURCES_aarch64_os_local aarch64/Los-freebsd.c aarch64/setcontext.S) is doing? Am I wrong, that second set will overwrite the previous one? So it's just ok to add aarch64/setcontext.S to the existing line with set?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, corrected the patch. Lets put them on separate lines for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New lines added.

Copy link
Member

Choose a reason for hiding this comment

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

I meant keep the existing style used in this file (one line added, zero removed).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I'm lost - there are few ways of doing set in this file. It's not possible to only add one line in here, as there's matching bracket on the line end when calling set, and this needs to go into the same variable which is included later?

Copy link
Member

Choose a reason for hiding this comment

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

The naming convention here is taken from libunwind, how they classify these files. Source are arch specific, others are OS specific and then few are machine independent (mi). Using the existing convention, that will become main...am11:runtime:patch-24.

Two lines because this is the first assembly file in aarch64 group. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, wasn't feeling good last days, now should be ok..

list(APPEND libunwind_coredump_la_SOURCES coredump/_UCD_access_reg_freebsd.c)
elseif(CLR_CMAKE_HOST_SUNOS)
set(libunwind_la_SOURCES_os ${libunwind_la_SOURCES_os_solaris})
Expand Down
Loading