[RFA] testsuite: Avoid auto-import warning on cygwin/mingw

[RFA] testsuite: Avoid auto-import warning on cygwin/mingw

[Pierre Muller]

Several tests fail on cygwin
because of:
gdb compile failed,
/usr/lib/gcc/i686-pc-cygwin/3.4.4/../../../../i686-pc-cygwin
/bin/ld: warning: auto-importing has been activated without
--enable-auto-import
 specified on the command line.
This should work unless it involves constant data structures referencing
symbols
 from auto-imported DLLs.Info: resolving _sg by linking to __imp__sg
(auto-impor
t)
UNTESTED: gdb.base/shlib-call.exp: Could not compile
/usr/local/src/gdbcvs/build
-normal/gdb/testsuite/gdb.base/shr1.sl,
/usr/local/src/gdbcvs/build-normal/gdb/t
estsuite/gdb.base/shr2.sl, or ../../../src/gdb/testsuite/gdb.base/shmain.c.

Adding --enable-auto-import to link stage
fixes this problem.

Is this OK?


Pierre Muller
Pascal language support maintainer for GDB


2009-10-02  Pierre Muller  <muller@ics.u-strasbg.fr>

	* lib/gdb.exp (gdb_compile): Add --enable-auto-import option for
	mingw and cygwin targets.


Index: lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.119
diff -u -p -r1.119 gdb.exp
--- lib/gdb.exp	13 Jul 2009 19:24:18 -0000	1.119
+++ lib/gdb.exp	1 Oct 2009 22:03:25 -0000
@@ -1731,7 +1731,10 @@ proc gdb_compile {source dest type optio
                     lappend options "additional_flags=-L${outdir}"
                 } elseif { [istarget "mips-sgi-irix*"] } {
                     lappend options "additional_flags=-rpath ${outdir}"
-                }
+                } elseif { ([istarget "*-*-mingw*"]
+			    || [istarget *-*-cygwin*]) } {
+		    lappend new_options
"additional_flags=-Wl,--enable-auto-import"
+		} 
             }
 	} elseif { $opt == "shlib_load" } {
 	    if { ([istarget "*-*-mingw*"]

Re: [RFA] testsuite: Avoid auto-import warning on cygwin/mingw

[Pedro Alves]

On Thursday 01 October 2009 23:09:42, Pierre Muller wrote:

> Adding --enable-auto-import to link stage
> fixes this problem.
> 
> Is this OK?
> 

I _think_ the idea is fine.  IWBN to have basic tests covering
printing auto-imported vs dllimported variables,
and covering stepping into import-lib'ed dll functions (through
the __imp_ trampolines).

> --- lib/gdb.exp	13 Jul 2009 19:24:18 -0000	1.119
> +++ lib/gdb.exp	1 Oct 2009 22:03:25 -0000
> @@ -1731,7 +1731,10 @@ proc gdb_compile {source dest type optio
>                      lappend options "additional_flags=-L${outdir}"
>                  } elseif { [istarget "mips-sgi-irix*"] } {
>                      lappend options "additional_flags=-rpath ${outdir}"
> -                }
> +                } elseif { ([istarget "*-*-mingw*"]
> +			    || [istarget *-*-cygwin*]) } {
> +		    lappend new_options
> "additional_flags=-Wl,--enable-auto-import"

Did you mean 'lappend options'?

> +		} 
>              }
>  	} elseif { $opt == "shlib_load" } {
>  	    if { ([istarget "*-*-mingw*"]
> 
> 

-- 
Pedro Alves

[RFA-v2] testsuite: Avoid auto-import warning on cygwin/mingw

[Pierre Muller]

> > Is this OK?
> >
> 
> I _think_ the idea is fine. 

  Thanks,

> IWBN to have basic tests covering
> printing auto-imported vs dllimported variables,
> and covering stepping into import-lib'ed dll functions (through
> the __imp_ trampolines).

  Not sure, I really get this, sorry.
 
> > --- lib/gdb.exp	13 Jul 2009 19:24:18 -0000	1.119
> > +++ lib/gdb.exp	1 Oct 2009 22:03:25 -0000
> > @@ -1731,7 +1731,10 @@ proc gdb_compile {source dest type optio
> >                      lappend options "additional_flags=-L${outdir}"
> >                  } elseif { [istarget "mips-sgi-irix*"] } {
> >                      lappend options "additional_flags=-rpath
> ${outdir}"
> > -                }
> > +                } elseif { ([istarget "*-*-mingw*"]
> > +			    || [istarget *-*-cygwin*]) } {
> > +		    lappend new_options
> > "additional_flags=-Wl,--enable-auto-import"
> 
> Did you mean 'lappend options'?
 
 No, appending to options
would mean that this addition would be parsed by the loop
  'foreach opt $options {' 
of line 1713,
while adding it to new_options like it
is done also below for 'shlib_load'.


  Furthermore, I always feel uncomfortable
when code inside a loop changes the variable 
on which the loop is performed (options in that case.)
  Wouldn't it be better to also
  append to new_options for the two conditions above?

So, what about this?


2009-10-02  Pierre Muller  <muller@ics.u-strasbg.fr>

	* lib/gdb.exp (gdb_compile): Append added options
	directly to new_options variable.
	Add --enable-auto-import option for
	mingw and cygwin targets.

Index: lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.119
diff -u -p -r1.119 gdb.exp
--- lib/gdb.exp	13 Jul 2009 19:24:18 -0000	1.119
+++ lib/gdb.exp	2 Oct 2009 07:26:54 -0000
@@ -1728,10 +1728,13 @@ proc gdb_compile {source dest type optio
                 if { ([test_compiler_info "gcc-*"]
                       &&  ([istarget "powerpc*-*-aix*"]
                         || [istarget "rs6000*-*-aix*"] )) } {
-                    lappend options "additional_flags=-L${outdir}"
+                    lappend new_options "additional_flags=-L${outdir}"
                 } elseif { [istarget "mips-sgi-irix*"] } {
-                    lappend options "additional_flags=-rpath ${outdir}"
-                }
+                    lappend new_options "additional_flags=-rpath ${outdir}"
+                } elseif { ([istarget "*-*-mingw*"]
+			    || [istarget *-*-cygwin*]) } {
+		    lappend new_options
"additional_flags=-Wl,--enable-auto-import"
+		} 
             }
 	} elseif { $opt == "shlib_load" } {
 	    if { ([istarget "*-*-mingw*"]

Re: [RFA-v2] testsuite: Avoid auto-import warning on cygwin/mingw

[Pedro Alves]

On Friday 02 October 2009 08:29:24, Pierre Muller wrote:
> 
> > Did you mean 'lappend options'?
>  
>  No, appending to options
> would mean that this addition would be parsed by the loop
>   'foreach opt $options {' 
> of line 1713,
> while adding it to new_options like it
> is done also below for 'shlib_load'.
> 
> 
>   Furthermore, I always feel uncomfortable
> when code inside a loop changes the variable 
> on which the loop is performed (options in that case.)
>   Wouldn't it be better to also
>   append to new_options for the two conditions above?
> 
> So, what about this?

Hmmm, I would have thought that foreach would still
only iterate over the original elements of $options (since foreach
takes the values of $options, not the variable reference),
no matter if you lappend to options; or am I confused, and
does it really keep iterating over newly append items?
It would seem like those "lappend options" are actually dead
code.  Otherwise, appending to `options' means that those
new items always ended on $new_options after
all the original options, but I don't think it matters
here.  Either patch looks ok to me, although you got me a
bit confused with tcl's foreach now.  :-)

-- 
Pedro Alves

Re: [RFA-v2] testsuite: Avoid auto-import warning on cygwin/mingw

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

>> 'foreach opt $options {' 

Pedro> Hmmm, I would have thought that foreach would still
Pedro> only iterate over the original elements of $options (since foreach
Pedro> takes the values of $options, not the variable reference),
Pedro> no matter if you lappend to options; or am I confused, and
Pedro> does it really keep iterating over newly append items?

You are not confused -- you are correct about this.
"$options" is substituted when the statement is parsed, not on each loop
iteration.

Tom

RE: [RFA-v2] testsuite: Avoid auto-import warning on cygwin/mingw

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Tom Tromey
> Envoyé : Friday, October 02, 2009 10:57 PM
> À : Pedro Alves
> Cc : Pierre Muller; gdb-patches@sourceware.org
> Objet : Re: [RFA-v2] testsuite: Avoid auto-import warning on
> cygwin/mingw
> 
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> >> 'foreach opt $options {'
> 
> Pedro> Hmmm, I would have thought that foreach would still
> Pedro> only iterate over the original elements of $options (since
> foreach
> Pedro> takes the values of $options, not the variable reference),
> Pedro> no matter if you lappend to options; or am I confused, and
> Pedro> does it really keep iterating over newly append items?
> 
> You are not confused -- you are correct about this.
> "$options" is substituted when the statement is parsed, not on each
> loop
> iteration.

This confirms that the current additions to options
are finally discarded by the
set options $new_options
a few lines down,
and that my patch is correct, no?

May I commit this?

Pierre

Re: [RFA-v2] testsuite: Avoid auto-import warning on cygwin/mingw

[Pedro Alves]

A Friday 02 October 2009 22:35:38, Pierre Muller escreveu:
> 
> > -----Message d'origine-----
> > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> > owner@sourceware.org] De la part de Tom Tromey
> > Envoyé : Friday, October 02, 2009 10:57 PM
> > À : Pedro Alves
> > Cc : Pierre Muller; gdb-patches@sourceware.org
> > Objet : Re: [RFA-v2] testsuite: Avoid auto-import warning on
> > cygwin/mingw
> > 
> > >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> > 
> > >> 'foreach opt $options {'
> > 
> > Pedro> Hmmm, I would have thought that foreach would still
> > Pedro> only iterate over the original elements of $options (since
> > foreach
> > Pedro> takes the values of $options, not the variable reference),
> > Pedro> no matter if you lappend to options; or am I confused, and
> > Pedro> does it really keep iterating over newly append items?
> > 
> > You are not confused -- you are correct about this.
> > "$options" is substituted when the statement is parsed, not on each
> > loop
> > iteration.
> 
> This confirms that the current additions to options
> are finally discarded by the
> set options $new_options
> a few lines down,
> and that my patch is correct, no?

Yes.  I would prefer then that the dead code isn't made
live and untested for no good reason.  I'd rather having it
removed, or if you prefer, go ahead apply the patch you
had before, the one that didn't touch the aix and irix bits, and
I'll remove the dead code afterwards.

-- 
Pedro Alves

RE: [RFA-v2] testsuite: Avoid auto-import warning on cygwin/mingw

[Pierre Muller]

> > This confirms that the current additions to options
> > are finally discarded by the
> > set options $new_options
> > a few lines down,
> > and that my patch is correct, no?
> 
> Yes.  I would prefer then that the dead code isn't made
> live and untested for no good reason.  I'd rather having it
> removed, or if you prefer, go ahead apply the patch you
> had before, the one that didn't touch the aix and irix bits, and
> I'll remove the dead code afterwards.

Ok, 

Pierre


For the record, here is what I finally committed:

$ cvs diff -up  ChangeLog commit.txt lib/gdb.exp
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.1979
diff -u -p -r1.1979 ChangeLog
--- ChangeLog   2 Oct 2009 21:28:34 -0000       1.1979
+++ ChangeLog   2 Oct 2009 22:22:38 -0000
@@ -1,4 +1,9 @@
 2009-10-02  Pierre Muller  <muller@ics.u-strasbg.fr>
+
+       * lib/gdb.exp (gdb_compile): Add --enable-auto-import option for
+       mingw and cygwin targets.
+
+2009-10-02  Pierre Muller  <muller@ics.u-strasbg.fr>
            Pedro Alves  <pedro@codesourcery.com>

        * lib/gdb.exp (gdb_compile): Avoid adding
Index: lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.120
diff -u -p -r1.120 gdb.exp
--- lib/gdb.exp 2 Oct 2009 21:28:34 -0000       1.120
+++ lib/gdb.exp 2 Oct 2009 22:22:39 -0000
@@ -1731,7 +1731,10 @@ proc gdb_compile {source dest type optio
                     lappend options "additional_flags=-L${outdir}"
                 } elseif { [istarget "mips-sgi-irix*"] } {
                     lappend options "additional_flags=-rpath ${outdir}"
-                }
+                } elseif { ([istarget "*-*-mingw*"]
+                           || [istarget *-*-cygwin*]) } {
+                   lappend new_options
"additional_flags=-Wl,--enable-auto-impo
rt"
+               }
             }
        } elseif { $opt == "shlib_load" } {
            if { ([istarget "*-*-mingw*"]