| Message ID | 52CBED25.7050705@synopsys.com |
|---|---|
| State | Superseded |
| Headers | show |
On 7 January 2014 13:03, Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote: > On Tuesday 07 January 2014 04:40 PM, Vineet Gupta wrote: >> On Tuesday 07 January 2014 02:59 PM, Bernhard Reutner-Fischer wrote: >>> On 3 January 2014 05:53, Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote: >>>> On Monday 23 December 2013 07:46 PM, Bernhard Reutner-Fischer wrote: >>>> >> ------------------->8----------------------------- >> ./extra/scripts/unifdef -B -t -f .//include/generated/unifdef_config.h -U_LIBC >> -U__UCLIBC_GEN_LOCALE -U__NO_CTYPE include/bits/uClibc_config.h >> #if !defined _FEATURES_H && !defined __need_uClibc_config_h >> # error Never include <bits/uClibc_config.h> directly; use <features.h> instead >> unifdef: include/bits/uClibc_config.h: 3: Inappropriate #endif >> unifdef: output may be truncated >> >> I debugged it a bit and it seems removing -t option to unifdef seems to elide the >> issue. >> Attached are my symbol file and the src file - see if u can reproduce it at your end. > > Following fixes the issue - although it might be incomplete. > > --------------------> > From 4f72593ad75f2c8a5a52501e020997f7d6e86aac Mon Sep 17 00:00:00 2001 > From: Vineet Gupta <vgupta@synopsys.com> > Date: Tue, 7 Jan 2014 17:25:14 +0530 > Subject: [PATCH] Reset unifdef state machine after a -f <file> parsing > > After commit 2a021ae81c36 "buildsys: update unifdef" there were sporadic > build failures at the time of uClibc header generation. > > ----------------->8------------------ > unifdef: include/bits/kernel_sigaction.h: 24: Inappropriate #endif > unifdef: output may be truncated > unifdef: include/bits/uClibc_config.h: 3: Inappropriate #endif > ... > ... > ----------------->8------------------ > > Turns out that unifdef now has -f <file> option to provide def/undef > symbols from a file. > > This helper file parsing uses the same code as the SRC file parsing. > However the parsing state machine uses global variables which need to > be "reset" after the -f pass. > > Signed-off-by: Vineet Gupta <vgupta@synopsys.com> > --- > extra/scripts/unifdef.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/extra/scripts/unifdef.c b/extra/scripts/unifdef.c > index b159df0a665b..f71ee66c896d 100644 > --- a/extra/scripts/unifdef.c > +++ b/extra/scripts/unifdef.c > @@ -378,6 +378,8 @@ processinout(const char *ifn, const char *ofn) > { > struct stat st; > > + incomment = linestate = 0; > + > if (ifn == NULL || strcmp(ifn, "-") == 0) { > filename = "[stdin]"; > linefile = NULL; > -- > 1.8.3.2 I have just pushed http://git.uclibc.org/uClibc/commit/?id=c71f8bc18e33da575c2f637a4dfa5e6bf120cd3c does that work for you, Vineet? Tony, when parsing defundefile there is an error in the linestate machine, it seems. For me, several entries are "swallowed", i.e. not parsed unless i conditionalize the fgets in skiphash on linestate. When the state machine is confused and leaves defundefile, processinout then starts with a confused state and throws away lines erroneously (which is what the hunk cited above would attempt to workaround after the fact, i guess). Please consider applying the 2 hunks in abovementioned commit c71f8bc or let me know if that is either wrong or you need me to submit a format patch. TIA and cheers,
On 7 January 2014 18:46, Tony Finch <dot@dotat.at> wrote: > Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: >> >> Tony, when parsing defundefile there is an error in the linestate >> machine, it seems. For me, several entries are "swallowed", i.e. not >> parsed unless i conditionalize the fgets in skiphash on linestate. >> When the state machine is confused and leaves defundefile, >> processinout then starts with a confused state and throws away lines >> erroneously (which is what the hunk cited above would attempt to >> workaround after the fact, i guess). >> >> Please consider applying the 2 hunks in abovementioned commit c71f8bc >> or let me know if that is either wrong or you need me to submit a >> format patch. > > Thanks for the bug report! I will investigate. Do you have some example > input that triggers this bug, that I could add to my test suite? full reproducer w/ the correct data generated by the c71f8bc fix is here: http://uclibc.org/~aldot/uClibc/unifdef-bug.tar.gz before that fix we failed to record e.g. the __EXTRA_WARNINGS__ undef: unifdef: addsym __UCLIBC_MALLOC_DEBUGGING__ undef unifdef: parser line 223 state NO comment DIRTY line unifdef: parser line 224 state NO comment START line unifdef: #define unifdef: addsym __WARNINGS__= unifdef: parser line 225 state NO comment DIRTY line unifdef: parser line 226 state NO comment START line unifdef: #undef unifdef: addsym __DOMULTI__ undef unifdef: parser line 227 state NO comment DIRTY line unifdef: parser line 228 state NO comment START line unifdef: addsym _LIBC undef unifdef: addsym __UCLIBC_GEN_LOCALE undef unifdef: addsym __NO_CTYPE undef unifdef: parser line 1 state C comment START line also note how the var of the __WARNINGS__ sym is empty.. thanks,
On 7 January 2014 19:25, Tony Finch <dot@dotat.at> wrote: > Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > >> full reproducer w/ the correct data generated by the c71f8bc fix is here: >> http://uclibc.org/~aldot/uClibc/unifdef-bug.tar.gz > > Awesome, thanks. I was having problems working out what was going wrong :-) I just see that this does not seem to be a proper fix this since the line-numbers in the defundefile are wrong now (which i do not care about though as we don't use the line-numbers here), so please have a closer look before you push that to the unifdef repo. TIA && cheers,
On 7 January 2014 19:37, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > On 7 January 2014 19:25, Tony Finch <dot@dotat.at> wrote: >> Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: >> >>> full reproducer w/ the correct data generated by the c71f8bc fix is here: >>> http://uclibc.org/~aldot/uClibc/unifdef-bug.tar.gz >> >> Awesome, thanks. I was having problems working out what was going wrong :-) > > I just see that this does not seem to be a proper fix this since the > line-numbers in the defundefile are wrong now (which i do not care > about though as we don't use the line-numbers here), so please have a > closer look before you push that to the unifdef repo. (like http://git.uclibc.org/uClibc/commit/?id=08e9f6f368dcd3bb95c9f68e666ca1a0fb5d6df6 fwiw) > > TIA && cheers,
On Tuesday 07 January 2014 10:48 PM, Bernhard Reutner-Fischer wrote: > On 7 January 2014 13:03, Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote: >> On Tuesday 07 January 2014 04:40 PM, Vineet Gupta wrote: >>> On Tuesday 07 January 2014 02:59 PM, Bernhard Reutner-Fischer wrote: >>>> On 3 January 2014 05:53, Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote: >>>>> On Monday 23 December 2013 07:46 PM, Bernhard Reutner-Fischer wrote: >>>>> >>> ------------------->8----------------------------- >>> ./extra/scripts/unifdef -B -t -f .//include/generated/unifdef_config.h -U_LIBC >>> -U__UCLIBC_GEN_LOCALE -U__NO_CTYPE include/bits/uClibc_config.h >>> #if !defined _FEATURES_H && !defined __need_uClibc_config_h >>> # error Never include <bits/uClibc_config.h> directly; use <features.h> instead >>> unifdef: include/bits/uClibc_config.h: 3: Inappropriate #endif >>> unifdef: output may be truncated >>> >>> I debugged it a bit and it seems removing -t option to unifdef seems to elide the >>> issue. >>> Attached are my symbol file and the src file - see if u can reproduce it at your end. >> Following fixes the issue - although it might be incomplete. >> >> --------------------> >> From 4f72593ad75f2c8a5a52501e020997f7d6e86aac Mon Sep 17 00:00:00 2001 >> From: Vineet Gupta <vgupta@synopsys.com> >> Date: Tue, 7 Jan 2014 17:25:14 +0530 >> Subject: [PATCH] Reset unifdef state machine after a -f <file> parsing >> >> After commit 2a021ae81c36 "buildsys: update unifdef" there were sporadic >> build failures at the time of uClibc header generation. >> >> ----------------->8------------------ >> unifdef: include/bits/kernel_sigaction.h: 24: Inappropriate #endif >> unifdef: output may be truncated >> unifdef: include/bits/uClibc_config.h: 3: Inappropriate #endif >> ... >> ... >> ----------------->8------------------ >> >> Turns out that unifdef now has -f <file> option to provide def/undef >> symbols from a file. >> >> This helper file parsing uses the same code as the SRC file parsing. >> However the parsing state machine uses global variables which need to >> be "reset" after the -f pass. >> >> Signed-off-by: Vineet Gupta <vgupta@synopsys.com> >> --- >> extra/scripts/unifdef.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/extra/scripts/unifdef.c b/extra/scripts/unifdef.c >> index b159df0a665b..f71ee66c896d 100644 >> --- a/extra/scripts/unifdef.c >> +++ b/extra/scripts/unifdef.c >> @@ -378,6 +378,8 @@ processinout(const char *ifn, const char *ofn) >> { >> struct stat st; >> >> + incomment = linestate = 0; >> + >> if (ifn == NULL || strcmp(ifn, "-") == 0) { >> filename = "[stdin]"; >> linefile = NULL; >> -- >> 1.8.3.2 > I have just pushed > http://git.uclibc.org/uClibc/commit/?id=c71f8bc18e33da575c2f637a4dfa5e6bf120cd3c > > does that work for you, Vineet? Yep fixes it for me. Thank. I was still concerned about clearing up of state machine from a -f <file> run so used a incomplete ifdef construct in sym file and now parser correctly bails out - so doesn't leave transient state behind for next run. -Vineet > > Tony, when parsing defundefile there is an error in the linestate > machine, it seems. For me, several entries are "swallowed", i.e. not > parsed unless i conditionalize the fgets in skiphash on linestate. > When the state machine is confused and leaves defundefile, > processinout then starts with a confused state and throws away lines > erroneously (which is what the hunk cited above would attempt to > workaround after the fact, i guess). > > Please consider applying the 2 hunks in abovementioned commit c71f8bc > or let me know if that is either wrong or you need me to submit a > format patch. > > TIA and cheers, >
diff --git a/extra/scripts/unifdef.c b/extra/scripts/unifdef.c index b159df0a665b..f71ee66c896d 100644 --- a/extra/scripts/unifdef.c +++ b/extra/scripts/unifdef.c @@ -378,6 +378,8 @@ processinout(const char *ifn, const char *ofn) { struct stat st; + incomment = linestate = 0; + if (ifn == NULL || strcmp(ifn, "-") == 0) { filename = "[stdin]"; linefile = NULL;