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

Simon Marchi (Code Review)
2019-10-30T16:36:44Z

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