Re: [fortran, patch] Fix handling of backtrace options in the library

2012-03-05 Thread Janne Blomqvist
On Sat, Mar 3, 2012 at 00:35, FX fxcoud...@gmail.com wrote:
 Hi all,

 I'll offer my first patch to the new 4.8 trunk. I noticed that the 
 -fbacktrace option and the GFORTRAN_ERROR_BACKTRACE environment variable are 
 somewhat inconsistently handled. Currently, the environment variabled doesn't 
 actually override the compilation option, as it should. So, I set to change 
 that (checking both options.backtrace and compile_options.backtrace instead 
 of only the latter) when I realized that we could improve this further by 
 moving the checking code: it is currently located in the set_options() 
 function, which is called from the main Fortran routine (call generated by 
 the front-end). This means that it's not triggered if the main program is C.

Good catch!

 Full disclosure: I removed the function maybe_find_addr2line() because it's 
 not needed anymore now the code was regrouped. However, I'll note that I 
 don't understand why it was useful before, and that I think the comment on 
 top of it was wrong (variable options could be seen in the function, it was 
 just shadowed by the local function argument!). Thus, while I was at it, I 
 renamed the options argument to set_options(), just to make sure we didn't 
 have a problem of this sort in the future.

I added it because, as you say, the argument options in
set_options() prevented access to the global variable options, and I
was lazy and considered a new function faster than renaming the
argument. :)

 The change was bootstrapped and regtested on x86_64-apple-darwin11, but is an 
 area not covered by the testsuite. I have manually checked that running 
 various types of abort, both in a pure Fortran code and a mixed C/Fortran 
 (with C main), with all combinations -fno-backtrace/GFORTRAN_ERROR_BACKTRACE, 
 behaved as expected.

I don't think it will work if you have a C main program which then
calls _gfortran_set_options() to enable backtracing per the
mixed-language programming chapter in the manual. You probably need to
move the setup code to a separate function which is called both from
init and set_options.

Also, in the patch as it stands now, you could make find_addr2line
static and remove the prototype from libgfortran.h.  That being said,
I'd recommend moving both find_addr2line and the signal handler setup
code to backtrace.c, and then do a bit of janitorial work checking
which functions and variables can be made static.

-- 
Janne Blomqvist


[fortran, patch] Fix handling of backtrace options in the library

2012-03-02 Thread FX
Hi all,

I'll offer my first patch to the new 4.8 trunk. I noticed that the -fbacktrace 
option and the GFORTRAN_ERROR_BACKTRACE environment variable are somewhat 
inconsistently handled. Currently, the environment variabled doesn't actually 
override the compilation option, as it should. So, I set to change that 
(checking both options.backtrace and compile_options.backtrace instead of only 
the latter) when I realized that we could improve this further by moving the 
checking code: it is currently located in the set_options() function, which is 
called from the main Fortran routine (call generated by the front-end). This 
means that it's not triggered if the main program is C. So, by moving it in the 
main initialization routine, we can ensure that an executable with a C main 
program but run using GFORTRAN_ERROR_BACKTRACE=Y will actually generate 
backtraces for Fortran errors, which I think is the cool thing to do.

Full disclosure: I removed the function maybe_find_addr2line() because it's not 
needed anymore now the code was regrouped. However, I'll note that I don't 
understand why it was useful before, and that I think the comment on top of it 
was wrong (variable options could be seen in the function, it was just 
shadowed by the local function argument!). Thus, while I was at it, I renamed 
the options argument to set_options(), just to make sure we didn't have a 
problem of this sort in the future.

The change was bootstrapped and regtested on x86_64-apple-darwin11, but is an 
area not covered by the testsuite. I have manually checked that running various 
types of abort, both in a pure Fortran code and a mixed C/Fortran (with C 
main), with all combinations -fno-backtrace/GFORTRAN_ERROR_BACKTRACE, behaved 
as expected.

OK to commit to trunk?
FX



traceback.diff
Description: Binary data


traceback.ChangeLog
Description: Binary data