| Message ID | 510D1D1E.7080705@myspectrum.nl |
|---|---|
| State | Not Applicable |
| Headers | show |
Dear Jeroen Hofstee, > Hello Albert, > > On 02/02/2013 12:32 PM, Albert ARIBAUD wrote: > >>> Sebastian wrote On 01.02.2013 08:55: > >>>> we are using u-boot in our embedded system with arm-1136jfs cpu. > >>>> We recently tried a new toolchain with GCC 4.7.2. > >>>> If compiled with the new toolchain the feature CONFIG_AUTO_COMPLETE > >>>> isn't working. > > > > [..] AFAIK it has never been seen on mainline code, > > The twister board from mainline / current master also has > this problem. I believe the mt_ventoux will have it as well, > but can't test it. > > > and has been very elusive for those who experience it -- enabling debug > > or simply adding/removing code makes it go away or reappear, leading me > > to thinking that some of the added code either does a data abort itself, > > or causes one in the mainline code it calls upon. > > yes, it is confusing. The following patch will e.g. make the > trap go away on the twister. Yet there is nothing wrong with the > original code it touches (or I fail to see what it is). > > Regards, > Jeroen > > diff --git a/common/command.c b/common/command.c > index 50c8429..520bd39 100644 > --- a/common/command.c > +++ b/common/command.c > @@ -185,7 +185,6 @@ static int complete_cmdv(int argc, char * const > argv[], char last_char, int maxv > cmd_tbl_t *cmdtp = ll_entry_start(cmd_tbl_t, cmd); > const int count = ll_entry_count(cmd_tbl_t, cmd); > const cmd_tbl_t *cmdend = cmdtp + count; > - const char *p; > int len, clen; > int n_found = 0; > const char *cmd; > @@ -224,11 +223,7 @@ static int complete_cmdv(int argc, char * const > argv[], char last_char, int maxv > * Some commands allow length modifiers (like "cp.b"); > * compare command name only until first dot. > */ > - p = strchr(cmd, '.'); > - if (p == NULL) > - len = strlen(cmd); > - else > - len = p - cmd; > + len = strlen(cmd); > > /* return the partial matches */ > for (; cmdtp != cmdend; cmdtp++) { Could it be that 'cmd' is possibly not zero-terminated string? Best regards, Marek Vasut
Dear Jeroen Hofstee, In message <510D1D1E.7080705@myspectrum.nl> you wrote: > > yes, it is confusing. The following patch will e.g. make the > trap go away on the twister. Yet there is nothing wrong with the > original code it touches (or I fail to see what it is). Note that your patch breaks commands that use length modifiers ... Best regards, Wolfgang Denk
Dear Marek Vasut, In message <201302021605.21681.marex@denx.de> you wrote: > > Could it be that 'cmd' is possibly not zero-terminated string? How would that ever happen? Best regards, Wolfgang Denk
On 02/02/2013 10:22 PM, Wolfgang Denk wrote: > Dear Jeroen Hofstee, > > In message <510D1D1E.7080705@myspectrum.nl> you wrote: >> yes, it is confusing. The following patch will e.g. make the >> trap go away on the twister. Yet there is nothing wrong with the >> original code it touches (or I fail to see what it is). > Note that your patch breaks commands that use length modifiers ... > > Best regards, > > Wolfgang Denk > I'm aware of that. This is not a patch to be applied, just to illustrate the weirdness encountered (it explicitly says the code is fine before this). This is resolved for now, see http://patchwork.ozlabs.org/patch/217695/ Regards, Jeroen
Hello, > So it seems to be, that patch at least solves this issue. > Sebastian: can you check if this is resolved also resolved for your board after applying http://patchwork.ozlabs.org/patch/217695/ Apperently we are still working with v2012.10. Could someone be so kind and provide a patch for v2012.10? We plan to upgrade to v2013.01, but not before the end of Februay. > Then this smells like a tool chain issue. You might contact Pengutronix support for help with their tool chain. We already asked Pengutronix. They use barebox with their toolchains and didn't have any problem with their new toolchain, yet. In their barebox.lds they have: __barebox_cmd_start = .; __barebox_cmd : { KEEP(*(SORT_BY_NAME(.barebox_cmd*))) } __barebox_cmd_end = .; And they thought __u_boot_cmd_start = .; .u_boot_cmd : { KEEP(*(.u_boot_cmd)) } __u_boot_cmd_end = .; would solve the problem. But it didn't. Best regards. Sebastian ========================================== CADCON Ingenieurgesellschaft mbH & Co. KG Geschaeftsfuehrer: Robert Bauer, Andreas Gundel Sitz der Gesellschaft: D-86368 Gersthofen Registergericht: Amtsgericht Augsburg HRA 14521 ========================================== -----Ursprüngliche Nachricht----- Von: Jeroen Hofstee [mailto:jeroen@myspectrum.nl] Gesendet: Samstag, 2. Februar 2013 22:45 An: Wolfgang Denk Cc: Jeroen Hofstee; Marek Vasut; Heiko Schocher; Priebe, Sebastian; u-boot@lists.denx.de Betreff: Re: [U-Boot] U-Boot Bug with newer GCC On 02/02/2013 10:22 PM, Wolfgang Denk wrote: > Dear Jeroen Hofstee, > > In message <510D1D1E.7080705@myspectrum.nl> you wrote: >> yes, it is confusing. The following patch will e.g. make the trap go >> away on the twister. Yet there is nothing wrong with the original >> code it touches (or I fail to see what it is). > Note that your patch breaks commands that use length modifiers ... > > Best regards, > > Wolfgang Denk > I'm aware of that. This is not a patch to be applied, just to illustrate the weirdness encountered (it explicitly says the code is fine before this). This is resolved for now, see http://patchwork.ozlabs.org/patch/217695/ Regards, Jeroen
Hi Sebastian, On Mon, 4 Feb 2013 07:11:30 +0000, "Priebe, Sebastian" <Sebastian.Priebe@cadcon.de> wrote: > Hello, > > > So it seems to be, that patch at least solves this issue. > > Sebastian: can you check if this is resolved also resolved for your board after applying http://patchwork.ozlabs.org/patch/217695/ > > Apperently we are still working with v2012.10. Could someone be so kind and provide a patch for v2012.10? > We plan to upgrade to v2013.01, but not before the end of Februay. Did you try to 'apply --reject' the patch to 2012.10 and see how this goes? > > Then this smells like a tool chain issue. You might contact Pengutronix support for help with their tool chain. > > We already asked Pengutronix. > They use barebox with their toolchains and didn't have any problem with their new toolchain, yet. > In their barebox.lds they have: > __barebox_cmd_start = .; > __barebox_cmd : { KEEP(*(SORT_BY_NAME(.barebox_cmd*))) } > __barebox_cmd_end = .; > > And they thought > __u_boot_cmd_start = .; > .u_boot_cmd : { KEEP(*(.u_boot_cmd)) } > __u_boot_cmd_end = .; > > would solve the problem. But it didn't. As long as symbols are defined at linker level it won't, I guess. My patch actively changes the way the commands start and end symbols are defined. > Best regards. > Sebastian Amicalement,
Hello, > Did you try to 'apply --reject' the patch to 2012.10 and see how this goes? This only adds a newline in command.c. Regards, Sebastian ========================================== CADCON Ingenieurgesellschaft mbH & Co. KG Geschaeftsfuehrer: Robert Bauer, Andreas Gundel Sitz der Gesellschaft: D-86368 Gersthofen Registergericht: Amtsgericht Augsburg HRA 14521 ========================================== -----Ursprüngliche Nachricht----- Von: Albert ARIBAUD [mailto:albert.u.boot@aribaud.net] Gesendet: Montag, 4. Februar 2013 09:49 An: Priebe, Sebastian Cc: Jeroen Hofstee; Wolfgang Denk; Marek Vasut; u-boot@lists.denx.de; Heiko Schocher Betreff: Re: [U-Boot] U-Boot Bug with newer GCC Hi Sebastian, On Mon, 4 Feb 2013 07:11:30 +0000, "Priebe, Sebastian" <Sebastian.Priebe@cadcon.de> wrote: > Hello, > > > So it seems to be, that patch at least solves this issue. > > Sebastian: can you check if this is resolved also resolved for your > > board after applying http://patchwork.ozlabs.org/patch/217695/ > > Apperently we are still working with v2012.10. Could someone be so kind and provide a patch for v2012.10? > We plan to upgrade to v2013.01, but not before the end of Februay. Did you try to 'apply --reject' the patch to 2012.10 and see how this goes? > > Then this smells like a tool chain issue. You might contact Pengutronix support for help with their tool chain. > > We already asked Pengutronix. > They use barebox with their toolchains and didn't have any problem with their new toolchain, yet. > In their barebox.lds they have: > __barebox_cmd_start = .; > __barebox_cmd : { KEEP(*(SORT_BY_NAME(.barebox_cmd*))) } > __barebox_cmd_end = .; > > And they thought > __u_boot_cmd_start = .; > .u_boot_cmd : { KEEP(*(.u_boot_cmd)) } > __u_boot_cmd_end = .; > > would solve the problem. But it didn't. As long as symbols are defined at linker level it won't, I guess. My patch actively changes the way the commands start and end symbols are defined. > Best regards. > Sebastian Amicalement, -- Albert.
Hi Sebastian, On Mon, 4 Feb 2013 11:28:26 +0000, "Priebe, Sebastian" <Sebastian.Priebe@cadcon.de> wrote: > Hello, > > > Did you try to 'apply --reject' the patch to 2012.10 and see how this goes? > > This only adds a newline in command.c. Do you mean the only change you see at all is this new line addition? I suspect there are other changes, which you missed, maybe because they apply fine. What does a 'git status' produce? Amicalement,
Hello, > Do you mean the only change you see at all is this new line addition? I suspect there are other changes, which you missed, maybe because they apply fine. What does a 'git status' produce? We are using svn, not git. There are too many differences. I can't apply the patch. There is no e.g. linker_list.h, etc... svn status after patching show only command/command.c changed. Greetings, Sebastian ========================================== CADCON Ingenieurgesellschaft mbH & Co. KG Geschaeftsfuehrer: Robert Bauer, Andreas Gundel Sitz der Gesellschaft: D-86368 Gersthofen Registergericht: Amtsgericht Augsburg HRA 14521 ========================================== -----Ursprüngliche Nachricht----- Von: Albert ARIBAUD [mailto:albert.u.boot@aribaud.net] Gesendet: Montag, 4. Februar 2013 12:36 An: Priebe, Sebastian Cc: Jeroen Hofstee; Wolfgang Denk; Marek Vasut; u-boot@lists.denx.de; Heiko Schocher Betreff: Re: [U-Boot] U-Boot Bug with newer GCC Hi Sebastian, On Mon, 4 Feb 2013 11:28:26 +0000, "Priebe, Sebastian" <Sebastian.Priebe@cadcon.de> wrote: > Hello, > > > Did you try to 'apply --reject' the patch to 2012.10 and see how this goes? > > This only adds a newline in command.c. Do you mean the only change you see at all is this new line addition? I suspect there are other changes, which you missed, maybe because they apply fine. What does a 'git status' produce? Amicalement, -- Albert.
Hi Sebastian, On Mon, 4 Feb 2013 14:23:17 +0000, "Priebe, Sebastian" <Sebastian.Priebe@cadcon.de> wrote: > Hello, > > > Do you mean the only change you see at all is this new line addition? I suspect there are other changes, which you missed, maybe because they apply fine. What does a 'git status' produce? > > We are using svn, not git. There are too many differences. I can't apply the patch. There is no e.g. linker_list.h, etc... > svn status after patching show only command/command.c changed. (maybe you should set your mailing tool to wrap at ~70 characters) If there is no linker_list.h, then the patch is basically not applicable -- you'd have to hand-port it by manually rewriting the linker file to include sections instead of defining symbols, something like .u_boot_cmd_start : { KEEP(*(.u_boot_cmd_start)) } .u_boot_cmd : { KEEP(*(.u_boot_cmd)) } .u_boot_cmd_end : { KEEP(*(.u_boot_cmd_end)) } And then, somewhere in a C file, define __u_boot_cmd_start as a 'struct {}' placed in section .u_boot_cmd_start, and __u_boot_cmd_end as a struct {} placed in section .u_boot_cmd_end. > Greetings, > Sebastian Amicalement,
Hello, > (maybe you should set your mailing tool to wrap at ~70 characters) Apperently I can't. The mailing tool (MS Outlook) setting are restricted by my companys IT. I have no "root" rights on my windows machine. I develop only on my linux machine. > If there is no linker_list.h, then the patch is basically not applicable -- you'd have to hand-port it by manually rewriting the linker file to include sections instead of defining symbols, something like I will wait until we upgraded to v2013.01 and apply the patch then. The problem seems to be solved, as several mailings said. I will work with the old toolchain until our upgrade. If the problem still exists after the upgrade I will scream again ;-) Thanks for the fast support. Sebastian ========================================== CADCON Ingenieurgesellschaft mbH & Co. KG Geschaeftsfuehrer: Robert Bauer, Andreas Gundel Sitz der Gesellschaft: D-86368 Gersthofen Registergericht: Amtsgericht Augsburg HRA 14521 ========================================== -----Ursprüngliche Nachricht----- Von: Albert ARIBAUD [mailto:albert.u.boot@aribaud.net] Gesendet: Montag, 4. Februar 2013 15:32 An: Priebe, Sebastian Cc: Jeroen Hofstee; Wolfgang Denk; Marek Vasut; u-boot@lists.denx.de; Heiko Schocher Betreff: Re: [U-Boot] U-Boot Bug with newer GCC Hi Sebastian, On Mon, 4 Feb 2013 14:23:17 +0000, "Priebe, Sebastian" <Sebastian.Priebe@cadcon.de> wrote: > Hello, > > > Do you mean the only change you see at all is this new line addition? I suspect there are other changes, which you missed, maybe because they apply fine. What does a 'git status' produce? > > We are using svn, not git. There are too many differences. I can't apply the patch. There is no e.g. linker_list.h, etc... > svn status after patching show only command/command.c changed. (maybe you should set your mailing tool to wrap at ~70 characters) If there is no linker_list.h, then the patch is basically not applicable -- you'd have to hand-port it by manually rewriting the linker file to include sections instead of defining symbols, something like .u_boot_cmd_start : { KEEP(*(.u_boot_cmd_start)) } .u_boot_cmd : { KEEP(*(.u_boot_cmd)) } .u_boot_cmd_end : { KEEP(*(.u_boot_cmd_end)) } And then, somewhere in a C file, define __u_boot_cmd_start as a 'struct {}' placed in section .u_boot_cmd_start, and __u_boot_cmd_end as a struct {} placed in section .u_boot_cmd_end. > Greetings, > Sebastian Amicalement, -- Albert.
Dear "Priebe, Sebastian", In message <E70AF999396FDF4EAE40E195B847096109F5F87F@SRVEXCH-2K10.CADCON.INTERN> you wrote: > > > (maybe you should set your mailing tool to wrap at ~70 characters) > > Apperently I can't. The mailing tool (MS Outlook) setting are restricted by= > my companys IT. > I have no "root" rights on my windows machine. I develop only on my linux m= > achine. Don't use this, then, to communicate with public mailing lists. There are many other options available, including many free ones. Best regards, Wolfgang Denk
diff --git a/common/command.c b/common/command.c index 50c8429..520bd39 100644 --- a/common/command.c +++ b/common/command.c @@ -185,7 +185,6 @@ static int complete_cmdv(int argc, char * const argv[], char last_char, int maxv cmd_tbl_t *cmdtp = ll_entry_start(cmd_tbl_t, cmd); const int count = ll_entry_count(cmd_tbl_t, cmd); const cmd_tbl_t *cmdend = cmdtp + count; - const char *p; int len, clen; int n_found = 0;