| Message ID | CAAgBjMkXuzG3KTx+viDn1+KoaWsgr3ahP2ga8dDyJhqHUgsjjA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR83775 | expand |
On 01/10/2018 12:01 PM, Prathamesh Kulkarni wrote: > Hi, > The attached patch tries to fix PR83775. > Validation in progress. > OK to commit if passes ? FWIW, the patch makes sense to me as it simplifies things for me when debugging using a cross-compiler. I reported the same ICE in bug 83775 and it was closed as WontFix but perhaps with a patch the decision will be reconsidered. It's not very user friendly to crash on the wrong/missing options. If the patch isn't accepted then perhaps one where the compiler issues an error would be. Martin
On 01/10/2018 12:21 PM, Martin Sebor wrote: > On 01/10/2018 12:01 PM, Prathamesh Kulkarni wrote: >> Hi, >> The attached patch tries to fix PR83775. >> Validation in progress. >> OK to commit if passes ? > > FWIW, the patch makes sense to me as it simplifies things for > me when debugging using a cross-compiler. I reported the same > ICE in bug 83775 and it was closed as WontFix but perhaps with > a patch the decision will be reconsidered. It's not very user > friendly to crash on the wrong/missing options. If the patch > isn't accepted then perhaps one where the compiler issues > an error would be. I've found the current behavior extremely user unfriendly as well. jeff
Hi Prathamesh, On 10/01/18 19:01, Prathamesh Kulkarni wrote: > Hi, > The attached patch tries to fix PR83775. > Validation in progress. > OK to commit if passes ? > I've read the relevant PRs and other's inputs on this (thanks for the perspectives!) I agree with Richard's design that the driver does all the smarts of processing the options to create the right -march string. There's a lot of options, extensions and variants that we need to keep track of and having it all in one place in the driver seems sensible as we need that information there to drive many other things like multilib selection and option string rewriting for the other parts of the toolchain. However, I do agree that being able to debug cc1 directly is indispensable when working with cross-compilers. I myself only have the capability of building just a cc1 for most non-arm (and aarch64) targets when I need to investigate issues on them. So I'm keen on fixing this segfault. At this point the end-user-facing experience really requires them to use the driver to ensure that the assembler, linker options are correct and the right libraries are selected. The .arch string we output is for the benefit of the assembler and if we the user wants the interface to the assembler to work correctly they should be using the driver anyway (A cc1 -> gas -> ld pipeline is not a supported pipeline AFAIK). So setting arch_to_print to the empty string here in order to not crash looks like a reasonable fix. Is there some way we can write an assert for the targ_options->x_arm_arch_string == NULL case to check that we are indeed invoked without a driver? If so, can you please add one? Otherwise add a comment saying that cc1 has been invoked directly. This patch is ok for trunk with either of those changes. Thanks, Kyrill > Thanks, > Prathamesh
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 196aa6de1ac..868251a154c 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -30954,7 +30954,10 @@ arm_declare_function_name (FILE *stream, const char *name, tree decl) /* Only update the assembler .arch string if it is distinct from the last such string we printed. */ - std::string arch_to_print = targ_options->x_arm_arch_string; + std::string arch_to_print; + if (targ_options->x_arm_arch_string) + arch_to_print = targ_options->x_arm_arch_string; + if (arch_to_print != arm_last_printed_arch_string) { std::string arch_name