[patch] coff-stgo32: Fix false format match

[patch] coff-stgo32: Fix false format match

[Jan Kratochvil]

Hello DJ,

on fedora-rawhide-x86_64 with -m32 (not on fedora-rawhide-i386) GDB gets false:
	FAIL: gdb.base/solib-nodir.exp: running to main in runto (end of file)
	FAIL: gdb.base/solib-nodir.exp: library loaded

because the ELF32 i386 file gets falsely matched as I386PTXMAGIC COFF file.
BFD/GDB then crashes on trying to read that file; fix of that crash is outside
of scope of this patch, it would not work with the COFF format anyway.

No regressions x86_64-fedora16pre-linux-gnu --target=i386-unknown-go32.


Thanks,
Jan


bfd/
2011-07-20  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix false coff-go32-exe matches.
	* coff-i386.c (TARGET_SYM) <_bfd_check_format>: Conditionally use
	COFF_CHECK_FORMAT.
	* coff-stgo32.c (go32_check_format): New forward declaration.
	(COFF_CHECK_FORMAT): New defintion.
	(go32_check_format): New function.

--- a/bfd/coff-i386.c
+++ b/bfd/coff-i386.c
@@ -671,8 +671,13 @@ const bfd_target
      bfd_getl16, bfd_getl_signed_16, bfd_putl16, /* hdrs */
 
 /* Note that we allow an object file to be treated as a core file as well.  */
-    {_bfd_dummy_target, coff_object_p, /* bfd_check_format */
-       bfd_generic_archive_p, coff_object_p},
+    /* bfd_check_format */
+#ifdef COFF_CHECK_FORMAT
+    {_bfd_dummy_target, COFF_CHECK_FORMAT,
+       bfd_generic_archive_p, COFF_CHECK_FORMAT},
+#else
+    {_bfd_dummy_target, coff_object_p, bfd_generic_archive_p, coff_object_p},
+#endif
     {bfd_false, coff_mkobject, _bfd_generic_mkarchive, /* bfd_set_format */
        bfd_false},
     {bfd_false, coff_write_object_contents, /* bfd_write_contents */
--- a/bfd/coff-stgo32.c
+++ b/bfd/coff-stgo32.c
@@ -95,6 +95,10 @@ create_go32_stub PARAMS ((bfd *));
 #define COFF_ADJUST_AUX_OUT_PRE adjust_aux_out_pre
 #define COFF_ADJUST_AUX_OUT_POST adjust_aux_out_post
 
+static const bfd_target *go32_check_format (bfd *abfd);
+
+#define COFF_CHECK_FORMAT go32_check_format
+
 static bfd_boolean
   go32_stubbed_coff_bfd_copy_private_bfd_data PARAMS ((bfd *, bfd *));
 
@@ -414,3 +418,23 @@ go32_stubbed_coff_bfd_copy_private_bfd_data  (ibfd, obfd)
 
   return TRUE;
 }
+
+/* coff_object_p only checks 2 bytes F_MAGIC at GO32_STUBSIZE inside the file
+   which is too fragile.  */
+
+static const bfd_target *
+go32_check_format (bfd *abfd)
+{
+  char mz[2];
+
+  if (bfd_bread (mz, 2, abfd) != 2 || mz[0] != 'M' || mz[1] != 'Z')
+    {
+      bfd_set_error (bfd_error_wrong_format);
+      return NULL;
+    }
+
+  if (bfd_seek (abfd, 0, SEEK_SET) != 0)
+    return NULL;
+
+  return coff_object_p (abfd);
+}


begin 755 solib-nodir.so.gz
M'XL("+;O)DX"`W-O;&EB+6YO9&ER+G-O`)U7?6@41Q2?O;O$BSGC);U\:%(\
M6BF)K=LDFO@)YL,D:F-BD]A:0EPNV35W>G>KMQM)Q%KC58@-`:FA?[3_"$(+
M%HN@E!8I:).6]I\VM/8#2JG4#TX,&C$M1<3K>S.S=W/K^4<[Q[LW;]]OYLW.
MO/=VWELM[:V2)!&K.>&'T@XG(:N!'U[`GJ\F?I)+*LFS9"E)HWG+882,>`G3
MHU#,R`,/?$"Y7.\`5@;ZLF)&,X01ZEV<Y@5:SLG2[;AIJI/0L:B",++TKX*>
M9&GB_#[ZKH2T=>PDRXYM2ER__,IOG??J>O\X>_)8WH-SAYP<Y^#CQIK\$IEN
MR,-GN"5Y0$TM/\V.&6/YEZZ_>_C7`[LJ/I^X7W#GQWL%=KL50O\Y0C+VSP_T
MHDU^'<A3PF0ON9_<A#(8?AYX$<DG>W']@GZW#0];3&XXF%P$*T75L,N2/80H
MRF!$CRJ&&8B9BD*44#1D$F4/,%`-#`>P&PB'#FE$V790Z=(&0X:IQ9K#`</0
M#+)'UTDXU!^1#5VNQ]X`ZRF:&C`#,$._8;"YX5%4)88.F)5170W%`$C:VK<V
M-2NU<HV\BN\O[DCZQZ3MS%5H,Y:%\A!YA,L^\!DW\`3P7!@PBQPFFD..!P<O
MO`">NY##\^[XK#N!`WM'IQ8\0\AX_)]D,GG\LIF3J,%IOL&_53`@/NWNG2(D
M>0$W*WG&S>TES\">D2!V;U^#D<DSN+(@JF_/@"B>]<X3-U_K3IR#WNC4^T4P
MY:SWZ*4RU`SUC<?GF=V"B;@?>B=<RQ.G@8_'T4,F+I1!?^*4#_ZK?K[RV!&_
M*ZV;,_P3G^+H^&7I!(4E)^.^U)AU<[%;7\=Q>BD^[>W=W3<%]KL3,C5?3LW[
MQD]5H-4?S.+Q^"-F/W^"/COA6YZ<B4_[>ONFQBN63YV$L8ER''KE6B$,?=--
M3/?X*$;*G9S/<!&`\P!.;.Q]OX#91J?.%N+>>J`/&VPNF;B`W8F++]#UWW4E
M9\9S03$TC^L4S\2`<8FIQS",[;]4[MQ0QGUC&*;`//0!\!X>M]1C#G41Z;!;
M*O>X7&-2.@=L!!S&"6DL<!]W-"W*40#P%?.EC4";0.\2SBO)SR\IG"/Z%_IA
M,X]5]$4,:AWCC,?6(AY3?SU.ZF422W?S/,?U`%\(_!V@?.['Z);SQ>R=O/P]
MX+5)*=!5!^M?=&!\LAP`;DI]''9%GW30]>FXECG@1T!^R.7_TS#G6FT%+&P]
MT&:8K*VY>;V_$C)BE7^U7"_7^&NK:VJJZVO7^"N[--6_)6"RYRMKJIZ"75-3
M9\>NJK(E`%G5^H<&T7;7L8WE1#9&(F:@'[@98SQH]:*ZJ<F#T2&Y?R@45E>&
M5$*E8,`($ED=B<)(QLT8TQS48D9(CV8("NAB6AAQK+,_;!*99CS9U(;AG^8]
M60LJ>V*!B*8$U5A:(O*`J<<,,,/8W@%08IJC4\5T:C\0"0V`3=VD?\P`384R
M9$*808]$M*A)5Z70EP^'HOO^PWF5\V^.P_9-M)KESQCV?X-?T.^KQ&@C]STB
M?/O6<K]T<%]%.LUU$N?HHPU\K(/[-M([@EW+_[:P^*%VT3>1?()=!Z>=W)^Q
MCSZ-Y+?9Q:;R[ZN#Q\1%CA/?`V,C(N`PAI"L6%LHX`[R^7-Y["+)6?;O@(#S
M.1E5VW!(AP4<WI.0*H5@M+Y;QP0<Y@JDI5GLCG$<W1?A3F/'O2?@SKL8[<B"
M.\UQ=+^*V5W,G07WD8!SXS?U*;A/!)P7<-Z<M$[$71!PF$-]3\%=YGOBY+D(
M[X(S@MXZMV^%^?!;G\AA^==^'M\).,RQ\SGI[X2(^T5\7[@;N'.SK^]W;M_)
M[U6>W.S[\B>?S_(1Q.VRQ0?2'=N=3\W-/%]+5RCX.+:C@'N8!2>E[DU\/1[F
M@SCG2T+\YMGFZUE,R(CTY'SV)EE^0_N+X<?\@\E>J`&8'S"YD*S@<<!B/9_F
M#=PW]OZEI!/Q)5:<EY(^O.\+^!"N6;"'\?K(9<D%Y&V;O9.8FUSI\1_R\V3S
MEY"/,5\*^B]Q/<+ZOP?^LJ"_Q?V5Z8O(`^$;*<%]^A'W*TLF/`>S>HGM8IED
MR>QD>J1T/45X_F0R,WS$8<FT:J*YD,FT2J(YC\FL`+N:DIDGWDC)>>S.XK3D
MA93O2,GYE%OW?R>/H,F4O(CR\RF9ER_%EKR8<G=*9MG-FY(+4W<F)A=EW#&<
M]";#XI?)+#+G4S+KN$LLF74\*;DTPS>=I,PF+R%%4F;]LT3*K'>629GUUP8I
ML][:)F765QU29GW5;9M?L8TWI,QZ"TJ@YI[.+J5]:W</EE?*Y@QI6[,@J+HR
M&-;[`V&%WC"4P-`P@0O#_K!F:JI<5U=;35"AA-1AE&H)NZ2H0Y'(B&6GI6,S
MG:NUJW%[2TI",U8_;64@907*N&%0A>K7ULN#FJGL'U#,X%!TG]P_;"W9&OXD
M9@`QJJ$KP4!4#4.=N/F-CL;M6YN)TM;>V=38KG2VMG:W]"@]C4WM+4JJ-,Q2
?8C8TB`6AK3+%4I/6D!E%99::E)6P_P)C1HRJS1``````
`
end

Re: [patch] coff-stgo32: Fix false format match

[DJ Delorie]

GO32 binaries need not have the stub on them, so you have to check for
MZ as well as a standard COFF magic number at offset zero.

Re: [patch] coff-stgo32: Fix false format match

[Jan Kratochvil]

On Wed, 20 Jul 2011 20:26:51 +0200, DJ Delorie wrote:
> GO32 binaries need not have the stub on them, so you have to check for
> MZ as well as a standard COFF magic number at offset zero.

I have modified only coff-stgo32.c ("coff-go32-exe") which seems to always
offset everything of COFF by GO32_STUBSIZE.

There is also coff-go32.c ("coff-go32") but I haven't touched that one.

My former patch for coff-go32-exe does MZ check && coff magic check.
The FSF binutils HEAD code does                    coff magic check.
Your proposed IIUC                     MZ check || coff magic check
is not useful because the coff magic check itself has the false positives.

Still I do not know whether there isn't in use some alternative env(GO32STUB)
for a .com file - where for .com there would be no 'MZ' signature.


Alternatively proposing the patch below.
No regressions x86_64-fedora16pre-linux-gnu --target=i386-unknown-go32.


Thanks,
Jan


bfd/
2011-07-20  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix false coff-go32-exe matches.
	* coff-i386.c (TARGET_SYM) <match_priority>: Use TARGET_MATCH_PRIORITY
	if available.
	* coff-stgo32.c (TARGET_MATCH_PRIORITY): New defintion.

--- a/bfd/coff-i386.c
+++ b/bfd/coff-i386.c
@@ -661,7 +661,11 @@ const bfd_target
 #endif
   '/',				/* ar_pad_char */
   15,				/* ar_max_namelen */
-  0,				/* match priority.  */
+#ifdef TARGET_MATCH_PRIORITY
+  TARGET_MATCH_PRIORITY,	/* match priority */
+#else
+  0,				/* match priority */
+#endif
 
   bfd_getl64, bfd_getl_signed_64, bfd_putl64,
      bfd_getl32, bfd_getl_signed_32, bfd_putl32,
--- a/bfd/coff-stgo32.c
+++ b/bfd/coff-stgo32.c
@@ -100,6 +100,10 @@ static bfd_boolean
 
 #define coff_bfd_copy_private_bfd_data go32_stubbed_coff_bfd_copy_private_bfd_data
 
+/* Two bytes of magic are too fragile when matched at GO32_STUBSIZE offset
+   inside executable.  */
+#define TARGET_MATCH_PRIORITY 10
+
 #include "coff-i386.c"
 
 /* This macro is used, because I cannot assume the endianness of the

Re: [patch] coff-stgo32: Fix false format match

[DJ Delorie]

Ah, right.  Nevermind.  Your original patch is fine, then.

Re: [patch] coff-stgo32: Fix false format match

[Jan Kratochvil]

On Wed, 20 Jul 2011 21:10:12 +0200, DJ Delorie wrote:
> Ah, right.  Nevermind.  Your original patch is fine, then.

On Wed, 20 Jul 2011 17:21:21 +0200, Jan Kratochvil wrote:
# bfd/
# 2011-07-20  Jan Kratochvil  <jan.kratochvil@redhat.com>
# 
# 	Fix false coff-go32-exe matches.
# 	* coff-i386.c (TARGET_SYM) <_bfd_check_format>: Conditionally use
# 	COFF_CHECK_FORMAT.
# 	* coff-stgo32.c (go32_check_format): New forward declaration.
# 	(COFF_CHECK_FORMAT): New defintion.
# 	(go32_check_format): New function.

Therefore checked the first patch in:
	http://sourceware.org/ml/binutils-cvs/2011-07/msg00109.html


Thanks,
Jan