[review v2] [gdb/testsuite] Compile ada with -lgnarl_pic and -lgnat_pic if required

["Simon Marchi (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32
......................................................................


Patch Set 2:

(3 comments)

I noted some nits, but it looks good to me in general. I'd still like the Ada maintainers to confirm that it is a valid workaround.

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/2/gdb/testsuite/lib/ada.exp 
File gdb/testsuite/lib/ada.exp:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/2/gdb/testsuite/lib/ada.exp@18 
PS2, Line 18: 
13 | # You should have received a copy of the GNU General Public License
14 | # along with this program.  If not, see <http://www.gnu.org/licenses/>.
15 | 
16 | 
17 | # Compile the ada code in $code to a file based on $name, using the flags
18 > # $compile_flag as well as debug, nowarning and quiet.
19 | # Return 1 if code can be compiled
20 | # Leave the file name of the resulting object in the upvar object.
21 | # Similar to gdb_simple_compile, but using gdb_compile_ada_1 instead of
22 | # gdb_compile.
23 | 

`$compile_flags`


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/2/gdb/testsuite/lib/ada.exp@60 
PS2, Line 60: 
55 | }
56 | 
57 | # Global variable used in gdb_ada_needs_libs_pic_suffix to detect recursion.
58 | # Initialize to 0.
59 | 
60 > global calculating_ada_needs_libs_pic_suffix
61 | set calculating_ada_needs_libs_pic_suffix 0
62 | 
63 | # Caching true/false for whether we need to use -lgnarl_pic -lgnat_pic in
64 | # target_compile_ada_from_dir.
65 | 

Is this global statement needed, since we're already in the global scope?


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/2/gdb/testsuite/lib/ada.exp@64 
PS2, Line 64: 
59 | 
60 | global calculating_ada_needs_libs_pic_suffix
61 | set calculating_ada_needs_libs_pic_suffix 0
62 | 
63 | # Caching true/false for whether we need to use -lgnarl_pic -lgnat_pic in
64 > # target_compile_ada_from_dir.
65 | 
66 | gdb_caching_proc gdb_ada_needs_libs_pic_suffix {
67 |     global calculating_ada_needs_libs_pic_suffix
68 |     if { $calculating_ada_needs_libs_pic_suffix } {
69 | 	return 0

Can you explain shortly here the intent of the method used to detect whether those flags are needed?  It seems to be: 

- try to compile without the flags, if it works it means we don't need the flags
- otherwise, try to compile with the flags, if it works it means we need the flags
- otherwise, we are not in a good situation (not able to build Ada files), so assume we don't need the flags, as they don't seem to help



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I3e1e40bd46236b45e2d7808c1cd744a075d4a148
Gerrit-Change-Number: 32
Gerrit-PatchSet: 2
Gerrit-Owner: Tom de Vries <tdevries@suse.de>
Gerrit-Reviewer: Joel Brobecker <brobecker@adacore.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-CC: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Wed, 30 Oct 2019 16:36:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment