Skip to content

Commit 64e5d52

Browse files
author
Craig Ringer
committed
Test code for clang stack pointer escape analyzer with sigsetjmp
Test various combos of sigsetjmp() paths and various configurations of guard variables + jump buffer handling using clang's stack escape checker https://clang.llvm.org/docs/analyzer/checkers.html#core-stackaddressescape-c following up on discussion on Pg list at https://www.postgresql.org/message-id/CAMsr+YE8tveiaGPuNzw+5fuo0yeZf7LePVLpx9MxUrFHMBG0gQ@mail.gmail.com Run make clean run_sigjmp_escape Includes additional test to hide it in a header and separate compilation unit. Run make clean run_sigjmp_escape_hdr
1 parent 5336151 commit 64e5d52

File tree

9 files changed

+452
-17
lines changed

9 files changed

+452
-17
lines changed

‎c/clang_return_stack_checks/.gitignore

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,10 @@
1+
*.o
12
guard.so
23
return_stack_escape
4+
sigjmp_escape
5+
sigjmp_escape_fake*
6+
sigjmp_escape_noinner
7+
sigjmp_escape_noinner_fake*
8+
sigjmp_escape_hdr
9+
sigjmp_escape_inner_pop
10+
scan-build-*/

‎c/clang_return_stack_checks/Makefile

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,72 @@
1+
default: check
2+
13
ifeq ($(origin CC),default)
2-
CC = scan-build clang
4+
CC = scan-build -o '$(SCAN_OUT)' clang
35
endif
46

7+
8+
# It doesn't seem to make much difference whether -O0 or -Og is used
9+
OPTIMIZE_FLAG ?= -O0
10+
#OPTIMIZE_FLAG ?= -Og
11+
512
ifeq ($(origin CFLAGS),undefined)
6-
CFLAGS = -Wall -Wextra -Og -ggdb3
13+
CFLAGS = -Wall -Wextra $(OPTIMIZE_FLAG) -ggdb3
714
endif
815

9-
all: return_stack_escape
16+
all: return_stack_escape sigjmp_escape
1017

11-
guard.so: guard.c guard.h
18+
ifndef SCAN_OUT
19+
tmpdir:
20+
$(eval SCAN_OUT := $(shell mktemp -d scan-build-XXXXXXXX))
21+
else
22+
tmpdir: ;
23+
endif
24+
25+
CFLAGS_LINK ?= $(CFLAGS) -fuse-ld=lld
26+
27+
guard.so: guard.c guard.h | tmpdir
1228
$(CC) $(CFLAGS) -shared -fPIC $< -o $@
1329

14-
return_stack_escape: return_stack_escape.c guard.so
30+
return_stack_escape: return_stack_escape.c guard.so | tmpdir
1531
$(CC) $(CFLAGS) $< ./guard.so -o $@
1632

33+
sigjmp_escape: sigjmp_escape.c guard.so | tmpdir
34+
$(CC) $(CFLAGS) -DUSE_INNER_GUARDS $< ./guard.so -o $@
35+
# These variants aren't run, but they exercise various combos for the static checker
36+
$(CC) $(CFLAGS) -DUSE_INNER_GUARDS -DFAKE_SIGSETJMP_RETURN=0 $< ./guard.so -o $@_fake0
37+
$(CC) $(CFLAGS) -DUSE_INNER_GUARDS -DFAKE_SIGSETJMP_RETURN=1 $< ./guard.so -o $@_fake1
38+
$(CC) $(CFLAGS) $< ./guard.so -o $@_noinner
39+
$(CC) $(CFLAGS) -DFAKE_SIGSETJMP_RETURN=0 $< ./guard.so -o $@_noinner_fake0
40+
$(CC) $(CFLAGS) -DFAKE_SIGSETJMP_RETURN=1 $< ./guard.so -o $@_noinner_fake1
41+
$(CC) $(CFLAGS) -DUSE_INNER_GUARDS -DPOP_ONLY_INNER_GUARDS $< ./guard.so -o $@_inner_pop
42+
43+
44+
45+
sigjmp_escape_hdr: sigjmp_escape_hdr.c sigjmp_escape_hdr_try.c | tmpdir
46+
$(CC) $(CFLAGS) -c sigjmp_escape_hdr_try.c -o sigjmp_escape_hdr_try.o
47+
$(CC) $(CFLAGS) -DUSE_FINALLY -c sigjmp_escape_hdr.c -o sigjmp_escape_hdr.o
48+
$(CC) $(CFLAGS_LINK) sigjmp_escape_hdr.o sigjmp_escape_hdr_try.o -o $@
49+
1750
clean:
18-
rm -f guard.so return_stack_escape
51+
rm -f *.o guard.so return_stack_escape sigjmp_escape sigjmp_escape_fake* sigjmp_escape_guard sigjmp_escape_guard_fake* sigjmp_escape_guard_inner_pop sigjmp_escape_hdr
52+
rm -rf scan-build-*
1953

20-
run:
54+
run_return_stack_escape: return_stack_escape
2155
./return_stack_escape 0
22-
if ./return_stack_escape; then exit 1; fi
56+
if ./return_stack_escape 1; then exit 1; fi
57+
58+
run_sigjmp_escape: sigjmp_escape
59+
./sigjmp_escape 0 0
60+
./sigjmp_escape 1 0
61+
if ./sigjmp_escape 0 1; then exit 1; fi
62+
if ./sigjmp_escape 1 1; then exit 1; fi
63+
64+
run_sigjmp_escape_hdr: sigjmp_escape_hdr
65+
./sigjmp_escape_hdr 0 0
66+
if ./sigjmp_escape_hdr 1 0; then exit 1; fi
67+
if ./sigjmp_escape_hdr 0 1; then exit 1; fi
68+
./sigjmp_escape_hdr 1 1
69+
70+
run: run_return_stack_escape run_sigjmp_escape run_sigjmp_escape_hdr
71+
72+
check: run

‎c/clang_return_stack_checks/guard.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,19 @@
11
#include "guard.h"
2+
#include <stdio.h>
23

34
struct guard * guard_ptr = 0;
5+
6+
int
7+
check_guard(void)
8+
{
9+
if (guard_ptr)
10+
{
11+
/* can't safely print guard name etc due to stack safety */
12+
fprintf(stderr, "guard not ok: guard pointer %p", guard_ptr);
13+
}
14+
else
15+
{
16+
fprintf(stderr, "guard ok: guard pointer empty");
17+
}
18+
return guard_ptr == 0;
19+
}

‎c/clang_return_stack_checks/guard.h

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,60 @@
11
#include <stdint.h>
22
#include <assert.h>
3+
#include <string.h>
4+
#include <stdio.h>
5+
6+
#define GUARD_NAME_MAX_LENGTH 20
7+
8+
#ifndef NO_TRACE_GUARDS
9+
#define TRACE_GUARDS
10+
#endif
311

412
struct guard
513
{
614
int8_t guard_set;
15+
const char gname[GUARD_NAME_MAX_LENGTH];
716
struct guard * previous;
817
};
918

1019
extern struct guard * guard_ptr;
1120

12-
static void
21+
static inline void
1322
set_guard(struct guard * const g)
1423
{
1524
assert(!g->guard_set);
1625
g->guard_set = 1;
1726
g->previous = guard_ptr;
1827
guard_ptr = g;
28+
#ifdef TRACE_GUARDS
29+
fprintf(stderr, " set_guard(%p=%s) pushed previous %p=%s\n",
30+
g, g->gname, g->previous,
31+
g->previous ? g->previous->gname : "(nil)");
32+
#endif
1933
}
2034

2135
static inline void
2236
clear_guard(struct guard * const g)
2337
{
38+
#ifdef TRACE_GUARDS
39+
fprintf(stderr, " clear_guard(%p=%s) restoring previous %p=%s\n",
40+
g, g->gname, g->previous,
41+
g->previous ? g->previous->gname : "(nil)");
42+
#endif
2443
assert(g->guard_set);
2544
assert(guard_ptr);
26-
g->guard_set = 0;
2745
assert(guard_ptr == g);
46+
g->guard_set = 0;
2847
guard_ptr = guard_ptr->previous;
2948
}
49+
50+
extern int check_guard(void);
51+
52+
#ifndef __has_attribute
53+
#define __has_attribute(attno) 0
54+
#endif
55+
56+
#if __has_attribute(unused)
57+
#define attr_unused() __attribute__((unused))
58+
#else
59+
#define attr_unused()
60+
#endif

‎c/clang_return_stack_checks/return_stack_escape.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,17 @@
77
int
88
foo(int do_fail)
99
{
10-
struct guard g = {0};
10+
struct guard g = {0, "g", 0};
1111
set_guard(&g);
1212

13+
/*
14+
* This should emit a warning from clang's scan-build like
15+
*
16+
* return_stack_escape.c:14:3: warning: Address of stack memory
17+
* associated with local variable 'g' is still referred to by the
18+
* global variable 'guard_ptr' upon returning to the caller. This will
19+
* be a dangling reference
20+
*/
1321
if (do_fail)
1422
return do_fail;
1523

@@ -32,12 +40,10 @@ main(int argc, char * argv[])
3240
if (*endpos != '\0')
3341
error(2, 0, "couldn't parse \"%s\" as an integer", argv[1]);
3442

35-
ret = foo(do_fail);
36-
37-
if (guard_ptr)
38-
printf("guard value: %hhd\n", guard_ptr->guard_set);
39-
else
40-
printf("guard value: no guard pointer\n");
43+
(void) foo(do_fail);
4144

45+
ret = !check_guard();
46+
fputc('\n', stderr);
4247
return ret;
48+
4349
}

‎c/clang_return_stack_checks/sigjmp_escape.c

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
#include <stdlib.h>
2+
#include <stdio.h>
3+
#include <setjmp.h>
4+
#include <error.h>
5+
6+
#include "guard.h"
7+
8+
/*
9+
* Support faking out sigsetjmp() to see if clang scan-build
10+
* behaves differently based on the call provided here.
11+
*/
12+
#ifdef FAKE_SIGSETJMP_RETURN
13+
#ifdef sigsetjmp
14+
#undef sigsetjmp
15+
#endif
16+
#define sigsetjmp(buf,clearsig) (FAKE_SIGSETJMP_RETURN)
17+
#endif /* FAKE_SIGSETJMP_RETURN */
18+
19+
static sigjmp_buf * jbuf = 0;
20+
21+
static void
22+
do_a_jump(void)
23+
{
24+
/* Jump back to the alternate branch */
25+
fprintf(stderr, " jumping to branch 1\n");
26+
siglongjmp(*jbuf, 1);
27+
}
28+
29+
static void
30+
test_jmp(int should_jump, int should_return_early)
31+
{
32+
struct guard g_outer = {0, "g_outer", 0};
33+
set_guard(&g_outer);
34+
35+
sigjmp_buf b;
36+
if (sigsetjmp(b, 0) == 0)
37+
{
38+
#ifdef USE_INNER_GUARDS
39+
struct guard g_branch_0 = {0, "g_branch_0", 0};
40+
set_guard(&g_branch_0);
41+
assert(guard_ptr->previous == &g_outer);
42+
#endif
43+
44+
jbuf = &b;
45+
46+
fprintf(stderr, " took branch 0\n");
47+
48+
if (should_jump)
49+
{
50+
#ifdef USE_INNER_GUARDS
51+
clear_guard(&g_branch_0);
52+
#endif
53+
do_a_jump();
54+
}
55+
56+
if (should_return_early)
57+
{
58+
59+
/*
60+
* Interestingly scan-build appears not to understand that when we
61+
* -DUSE_INNER_GUARDS and run with should_jump=0, the pointer to
62+
* g_outer stored in g_branch_0->previous by
63+
* set_guard(&g_branch_0) has still escaped. It either didn't
64+
* follow the indirection, or only reports the escape of
65+
* g_branch_0. We can check which by optionally popping only one
66+
* level of guards:
67+
*/
68+
#if defined(USE_INNER_GUARDS) && defined(POP_ONLY_INNER_GUARDS)
69+
fprintf(stderr, " clearing g_branch_0 before early return\n");
70+
clear_guard(&g_branch_0);
71+
assert(guard_ptr == &g_outer);
72+
#endif
73+
74+
/*
75+
* Deliberately fail to clear guards guard_outer or g_branch_0 here.
76+
*
77+
* Should raise clang scan-build warning
78+
* warning: Address of stack memory associated with local variable 'b' is still referred to by the global variable 'jbuf' upon returning to the caller. This will be a dangling reference
79+
*
80+
* and if USE_INNER_GUARDS then
81+
* warning: Address of stack memory associated with local variable 'g_branch_0' is still referred to by the global variable 'guard_ptr' upon returning to the caller. This will be a dangling reference
82+
*
83+
* otherwise the same complaint about g_outer.
84+
*/
85+
fprintf(stderr, " escaping branch 0 early\n");
86+
return;
87+
}
88+
89+
#ifdef USE_INNER_GUARDS
90+
clear_guard(&g_branch_0);
91+
#endif
92+
}
93+
else
94+
{
95+
#ifdef USE_INNER_GUARDS
96+
struct guard g_branch_1 = {0, "g_branch_1", 0};
97+
set_guard(&g_branch_1);
98+
#endif
99+
100+
/* entered with should_jump == 1 and took the do_a_jump() branch */
101+
fprintf(stderr, " took branch 1\n");
102+
103+
if (should_return_early)
104+
{
105+
#if defined(USE_INNER_GUARDS) && defined(POP_ONLY_INNER_GUARDS)
106+
/*
107+
* Like the branch0 case, we'll also check what happens if we pop
108+
* only one level of guards.
109+
*/
110+
fprintf(stderr, " clearing g_branch_1 before early return\n");
111+
clear_guard(&g_branch_1);
112+
assert(guard_ptr == &g_outer);
113+
#endif
114+
115+
/*
116+
* Deliberately fail to clear guards guard_outer or g_branch_1 here.
117+
*
118+
* If -DUSE_INNER_GUARDS, should raise clang scan-build warning:
119+
*
120+
* warning: Address of stack memory associated with local variable 'g_branch_1' is still referred to by the global variable 'guard_ptr' upon returning to the caller. This will be a dangling reference
121+
*
122+
* Otherwise it should complain about g_outer.
123+
*/
124+
125+
fprintf(stderr, " escaping branch 1 early\n");
126+
return;
127+
}
128+
129+
#ifdef USE_INNER_GUARDS
130+
clear_guard(&g_branch_1);
131+
#endif
132+
}
133+
134+
/* Properly clear stored jump buffer */
135+
jbuf = 0;
136+
137+
/*
138+
* scan-build fails to notice that no path reaches this code when
139+
* called with should_return_early=1 (for either value of should_jump),
140+
* so it does not detect the escape of g_outer.
141+
*
142+
* clang should be warning us when we can escape with a leaked guard
143+
* pointer, but it seems like it gets confused by sigsetjmp.
144+
*/
145+
clear_guard(&g_outer);
146+
}
147+
148+
int
149+
main(int argc, char * argv[])
150+
{
151+
int should_jump;
152+
int should_return_early;
153+
int guard_ok;
154+
if (argc != 3)
155+
error(2, 0, "usage: %s {{should_jump 0|1}} {{should_return_early 0|1}}", argv[0]);
156+
should_jump = atoi(argv[1]);
157+
should_return_early = atoi(argv[2]);
158+
fprintf(stderr, "sigjmp_escape(%d,%d):\n",
159+
should_jump, should_return_early);
160+
test_jmp(should_jump, should_return_early);
161+
guard_ok = check_guard();
162+
fputs("\n", stderr);
163+
if (jbuf != 0)
164+
error(1, 0, " escaped with invalid sigjmp_buf");
165+
else if (!guard_ok)
166+
error(1, 0, " escaped with bad guard state");
167+
else
168+
fprintf(stderr, " ok\n");
169+
170+
return 0;
171+
}

0 commit comments

Comments
 (0)