Skip to content

PS-10970 fix clang volatile warnings #15

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.

Sign up for GitHub

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

Open
lukin-oleksiy wants to merge 1 commit into
base: master
Choose a base branch
from
Open

PS-10970 fix clang volatile warnings #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

add_library(
coredumper
STATIC
Expand All @@ -18,6 +17,12 @@ set_target_properties(
"../include/coredumper/coredumper.h"
)

target_compile_options(
coredumper PRIVATE
$<$<COMPILE_LANG_AND_ID:CXX,Clang,AppleClang>:-Wdeprecated-volatile>
$<$<COMPILE_LANG_AND_ID:CXX,GNU>:-Wvolatile>

@inikep inikep May 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This may require a quite new cmake version. Please check it.

)

target_link_libraries(coredumper ${CMAKE_THREAD_LIBS_INIT})

export(TARGETS coredumper FILE "coredumper.cmake")
Expand Down
3 changes: 3 additions & 0 deletions src/elfcore.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ typedef struct fpxregs { /* SSE registers */
#define FPREGS fpxregs
#else
typedef struct fpxregs { /* x86-64 stores FPU registers in SSE struct */
uint8_t unused;
} fpxregs;
typedef struct fpregs { /* FPU registers */
#define FPREGS fpregs
Expand All @@ -123,6 +124,7 @@ typedef struct fpregs { /* FPU registers */
#define regs i386_regs /* General purpose registers */
#elif defined(__ARM_ARCH_3__)
typedef struct fpxregs { /* No extended FPU registers on ARM */
uint8_t unused;

@kamil-holubicki kamil-holubicki May 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I didn't analyze the whole thing, but see that there are places like memcpy(fpx, scratch, sizeof(struct fpxregs)); in code...

} fpxregs;
typedef struct fpregs { /* FPU registers */
struct fp_reg {
Expand All @@ -142,6 +144,7 @@ typedef struct fpregs { /* FPU registers */
#define regs arm_regs /* General purpose registers */
#elif defined(__mips__)
typedef struct fpxregs { /* No extended FPU registers on MIPS */
uint8_t unused;
} fpxregs;
typedef struct fpregs {
uint64_t fpuregs[32];
Expand Down
10 changes: 7 additions & 3 deletions src/linuxthreads.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,14 @@ static volatile int *sig_pids, sig_num_threads, sig_proc, sig_marker;
static void SignalHandler(int signum, siginfo_t *si, void *data) {
if (sig_pids != NULL) {
if (signum == SIGABRT) {
while (sig_num_threads-- > 0) {
while (sig_num_threads > 0) {
const int next_thread = sig_num_threads - 1;
sig_num_threads = next_thread;
/* Not sure if sched_yield is really necessary here, but it does not */
/* hurt, and it might be necessary for the same reasons that we have */
/* to do so in sys_ptrace_detach(). */
sys_sched_yield();
sys_ptrace(PTRACE_KILL, sig_pids[sig_num_threads], 0, 0);
sys_ptrace(PTRACE_KILL, sig_pids[next_thread], 0, 0);

@inikep inikep May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The issue here seems to be sig_num_threads-- for sig_num_threads=0.
Isn't it much easier to use:

while (sig_num_threads > 0) {
sys_sched_yield();
sys_ptrace(PTRACE_KILL, sig_pids[sig_num_threads], 0, 0);
sig_num_threads--;
}

}
} else if (sig_num_threads > 0) {
ResumeAllProcessThreads(sig_num_threads, (int *)sig_pids);
Expand Down Expand Up @@ -306,7 +308,9 @@ static void ListerThread(struct ListerParams *args) {
* check there first, and then fall back on the older naming
* convention if necessary.
*/
if ((sig_proc = proc = c_open(*proc_path, O_RDONLY | O_DIRECTORY, 0)) < 0) {
proc = c_open(*proc_path, O_RDONLY | O_DIRECTORY, 0);
sig_proc = proc;
if (proc < 0) {
if (*++proc_path != NULL) continue;
goto failure;
}
Expand Down