jreiser at BitWagon dot com
2018-02-21 18:22:46 UTC
https://sourceware.org/bugzilla/show_bug.cgi?id=22873
Bug ID: 22873
Summary: ada-lex.c: races and non-robust build rules
Product: gdb
Version: HEAD
Status: UNCONFIRMED
Severity: normal
Priority: P2
Component: build
Assignee: unassigned at sourceware dot org
Reporter: jreiser at BitWagon dot com
Target Milestone: ---
The build rules for ada-lex.c have races and are not robust in the face of
common situations. They are a barrier for contributing to gdb.
In the generated gdb/Makefile:
.PRECIOUS: ada-lex.c
means that most variations of "make clean" do not remove ada-lex.c. The
exception is "make local-maintainer-clean" which requires extra learning by a
newbie whose Linux distribution demands "report all gdb bugs upstream [i.e., at
sourceware.org]."
Why does it matter? Because of races in the building of ada-lex.c:
=====
%.c: %.l
if [ "$(FLEX)" ] && $(FLEX) --version >/dev/null 2>&1; then \
$(FLEX) -o$@ $< && \
rm -f $@.new && \
sed -e '/extern.*malloc/d' \
-e '/extern.*realloc/d' \
-e '/extern.*free/d' \
-e '/include.*malloc.h/d' \
-e 's/\([^x]\)malloc/\1xmalloc/g' \
-e 's/\([^x]\)realloc/\1xrealloc/g' \
-e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
-e 's/\([ \t;,(]\)free$$/\1xfree/g' \
-e 's/yy_flex_xrealloc/yyxrealloc/g' \
< $@ > $@.new && \
rm -f $@ && \
mv $@.new $@; \
=====
The first thing that "flex -oada-lex.c ..." does is open("ada-lex.c", 0666,
O_CREAT). So if the user types ^C (SIGINT) after the open() but before flex
does any write() to the file [flex thinks for a while before writing], then the
result is an ada-lex.c file that exists but is empty (has .st_size of 0). This
causes gdb/ada-lex.y to be incomplete, and subsequent compilations will fail
because 'yyin' is undefined. "make clean" will not help because ada-lex.c is
.PRECIOUS. If you haven't seen this before and do not stumble onto
'local-maintainer-clean' then apparently the only way forward is to start over
from the very beginning. Any file of zero length should not be .PRECIOUS.
But wait, there's more! Any SIGINT after flex and before the rm will leave
ada-lex.c exactly as flex wrote it. All the processing by sed will be ignored,
and the original ada-lex.c result of flex will persist because ada-lex.c is
.PRECIOUS.
Finally in the last two commands:
rm -f $@ && \
mv $@.new $@
the 'rm' is superfluous and should not be used. Just the 'mv' by itself has
the same effect, does it atomically, and has exactly the same error conditions.
Looking at the rule as a whole and in general, it is poor practice to generate
a file that has the name of the end result (ada-lex.c: "$(FLEX) -o$@ ")
anywhere except at the very last step.
[Somewhat less important: why not pipe "flex --stdout ... | sed ..." ?]
Bug ID: 22873
Summary: ada-lex.c: races and non-robust build rules
Product: gdb
Version: HEAD
Status: UNCONFIRMED
Severity: normal
Priority: P2
Component: build
Assignee: unassigned at sourceware dot org
Reporter: jreiser at BitWagon dot com
Target Milestone: ---
The build rules for ada-lex.c have races and are not robust in the face of
common situations. They are a barrier for contributing to gdb.
In the generated gdb/Makefile:
.PRECIOUS: ada-lex.c
means that most variations of "make clean" do not remove ada-lex.c. The
exception is "make local-maintainer-clean" which requires extra learning by a
newbie whose Linux distribution demands "report all gdb bugs upstream [i.e., at
sourceware.org]."
Why does it matter? Because of races in the building of ada-lex.c:
=====
%.c: %.l
if [ "$(FLEX)" ] && $(FLEX) --version >/dev/null 2>&1; then \
$(FLEX) -o$@ $< && \
rm -f $@.new && \
sed -e '/extern.*malloc/d' \
-e '/extern.*realloc/d' \
-e '/extern.*free/d' \
-e '/include.*malloc.h/d' \
-e 's/\([^x]\)malloc/\1xmalloc/g' \
-e 's/\([^x]\)realloc/\1xrealloc/g' \
-e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
-e 's/\([ \t;,(]\)free$$/\1xfree/g' \
-e 's/yy_flex_xrealloc/yyxrealloc/g' \
< $@ > $@.new && \
rm -f $@ && \
mv $@.new $@; \
=====
The first thing that "flex -oada-lex.c ..." does is open("ada-lex.c", 0666,
O_CREAT). So if the user types ^C (SIGINT) after the open() but before flex
does any write() to the file [flex thinks for a while before writing], then the
result is an ada-lex.c file that exists but is empty (has .st_size of 0). This
causes gdb/ada-lex.y to be incomplete, and subsequent compilations will fail
because 'yyin' is undefined. "make clean" will not help because ada-lex.c is
.PRECIOUS. If you haven't seen this before and do not stumble onto
'local-maintainer-clean' then apparently the only way forward is to start over
from the very beginning. Any file of zero length should not be .PRECIOUS.
But wait, there's more! Any SIGINT after flex and before the rm will leave
ada-lex.c exactly as flex wrote it. All the processing by sed will be ignored,
and the original ada-lex.c result of flex will persist because ada-lex.c is
.PRECIOUS.
Finally in the last two commands:
rm -f $@ && \
mv $@.new $@
the 'rm' is superfluous and should not be used. Just the 'mv' by itself has
the same effect, does it atomically, and has exactly the same error conditions.
Looking at the rule as a whole and in general, it is poor practice to generate
a file that has the name of the end result (ada-lex.c: "$(FLEX) -o$@ ")
anywhere except at the very last step.
[Somewhat less important: why not pipe "flex --stdout ... | sed ..." ?]
--
You are receiving this mail because:
You are on the CC list for the bug.
You are receiving this mail because:
You are on the CC list for the bug.