Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[Sedat Dilek]

Hi,

my build of linux-next (next-20110308, the same with the one from
yesterday) is broken.
(I translated the German output.)

[ build.log ]
  AS      arch/x86/kernel/entry_32.o
/home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
Assembler messages:
/home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
Error: .size expression does not evaluate to a constant
make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
make[4]: *** [arch/x86] Fehler 2 (Error 2)
make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
unfinished jobs...)

I am not sure if this is a problem of Debian's binutils snapshot from
binutils-2_21-branch (Debian-version: 2.21.0.20110302-1) from sid/i386
or this is only a problem for x86, but I just want to let you know.

FYI: The previous binutils (2.21.0.20110216-2) works fine.

I have tried with reverting the last two changes to
arch/x86/kernel/entry_32.S in linux-next:

"x86: Use {push,pop}_cfi in more places" (see [1])
"x86, asm: Cleanup unnecssary macros in asm-offsets.c" (see [2])

Reverting both or [1] or [2] breaks with Debians as (2.21.0.20110302-1).

BTW, [3] has a complete GIT history for the above file.

So, I am unsure from where the problem exactly aroses.
If this a known issue (and a fix around) or rings a bell to you, let
me and others know.

Thanks in advance.

Regards,
- Sedat -

[1] http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=60cf637a13932a4750da6746efd0199e8a4c341b

[2] http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=7bf04be8f48ceeeffa5b5a79734d6d6e0d59e5f8

[3] http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=history;f=arch/x86/kernel/entry_32.S;h=2878821cb8c1da1d7147b26271114fa9546afe03;hb=HEAD

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[Sedat Dilek]

[-- Attachment #1: Type: text/plain, Size: 3069 bytes --]

On 3/8/11, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> Hi,
>
> my build of linux-next (next-20110308, the same with the one from
> yesterday) is broken.
> (I translated the German output.)
>
> [ build.log ]
>   AS      arch/x86/kernel/entry_32.o
> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
> Assembler messages:
> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
> Error: .size expression does not evaluate to a constant
> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
> make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
> make[4]: *** [arch/x86] Fehler 2 (Error 2)
> make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
> unfinished jobs...)
>
> I am not sure if this is a problem of Debian's binutils snapshot from
> binutils-2_21-branch (Debian-version: 2.21.0.20110302-1) from sid/i386
> or this is only a problem for x86, but I just want to let you know.
>
> FYI: The previous binutils (2.21.0.20110216-2) works fine.
>
> I have tried with reverting the last two changes to
> arch/x86/kernel/entry_32.S in linux-next:
>
> "x86: Use {push,pop}_cfi in more places" (see [1])
> "x86, asm: Cleanup unnecssary macros in asm-offsets.c" (see [2])
>
> Reverting both or [1] or [2] breaks with Debians as (2.21.0.20110302-1).
>
> BTW, [3] has a complete GIT history for the above file.
>
> So, I am unsure from where the problem exactly aroses.
> If this a known issue (and a fix around) or rings a bell to you, let
> me and others know.
>
> Thanks in advance.
>
> Regards,
> - Sedat -
>
> [1]
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=60cf637a13932a4750da6746efd0199e8a4c341b
>
> [2]
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=7bf04be8f48ceeeffa5b5a79734d6d6e0d59e5f8
>
> [3]
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=history;f=arch/x86/kernel/entry_32.S;h=2878821cb8c1da1d7147b26271114fa9546afe03;hb=HEAD
>

I just jumped on binutils-2_21-branch GIT and looked what changed in
gas/ directory.

So there were two patches to consider in the time 2011-03-02 and
2011-02-16 (chronological order, latest listed as first):

[1] commit a57ef8e7f3b214e0cf9ee33bb17c11f9f63bf6aa
"* symbols.c (report_op_error): Remove unnecessary forward declaration."

[2] commit 345bbf7731af2912390e72b86807eb1b2af3e27b
"PR gas/12519"

Reverting [1] still breaks build.
With reverting [1] and [2] I had success.
(Unfortunately, both patches have made changes to gas/ChangeLog, so
for isolating only [2] I had no fun and time).

As a conclusion:
It looks like the "PR gas/12519" patch breaks the linux-next kernel
(or l-n needs some modifications?).
It's up to the experts.

Hope this helps to fix the problem.

- Sedat -

P.S.: I have added both revert-patches.

[1] http://sourceware.org/git/?p=binutils.git;a=commit;h=a57ef8e7f3b214e0cf9ee33bb17c11f9f63bf6aa

[2] http://sourceware.org/git/?p=binutils.git;a=commit;h=345bbf7731af2912390e72b86807eb1b2af3e27b

[-- Attachment #2: 0001-Revert-symbols.c-report_op_error-Remove-unnecessary-.patch --]
[-- Type: text/x-patch, Size: 7107 bytes --]

From a961491daceb17d4d060e1a51025b1eb9e2e32cf Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@gmail.com>
Date: Tue, 8 Mar 2011 11:54:39 +0100
Subject: [PATCH 1/2] Revert "	* symbols.c (report_op_error): Remove unnecessary forward declaration."

This reverts commit a57ef8e7f3b214e0cf9ee33bb17c11f9f63bf6aa.
---
 gas/ChangeLog |    8 -----
 gas/symbols.c |   99 ++++++++++++++++++++++++++-------------------------------
 2 files changed, 45 insertions(+), 62 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 488d7b4..caa9b1e 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,11 +1,3 @@
-2011-02-28  Alan Modra  <amodra@gmail.com>
-
-	* symbols.c (report_op_error): Remove unnecessary forward declaration.
-	Add "op" parameter.  Report operator and operand segments in error
-	message, not operand symbols.
-	(resolve_symbol_value): Always set segment for equated symbols, not
-	just when finalizing.  Adjust report_op_error calls.
-
 2011-02-25  Alan Modra  <amodra@gmail.com>
 
 	PR gas/12519
diff --git a/gas/symbols.c b/gas/symbols.c
index 91d0cdb..9a4e2be 100644
--- a/gas/symbols.c
+++ b/gas/symbols.c
@@ -1,7 +1,7 @@
 /* symbols.c -symbol table-
    Copyright 1987, 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998,
-   1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
-   2011 Free Software Foundation, Inc.
+   1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009
+   Free Software Foundation, Inc.
 
    This file is part of GAS, the GNU Assembler.
 
@@ -72,6 +72,7 @@ static long dollar_label_instance (long);
 static long fb_label_instance (long);
 
 static void print_binary (FILE *, const char *, expressionS *);
+static void report_op_error (symbolS *, symbolS *, symbolS *);
 
 /* Return a pointer to a new symbol.  Die if we can't make a new
    symbol.  Fill in the symbol's values.  Add symbol to end of symbol
@@ -969,65 +970,55 @@ use_complex_relocs_for (symbolS * symp)
 #endif
 
 static void
-report_op_error (symbolS *symp, symbolS *left, operatorT op, symbolS *right)
+report_op_error (symbolS *symp, symbolS *left, symbolS *right)
 {
   char *file;
   unsigned int line;
-  segT seg_left = left ? S_GET_SEGMENT (left) : 0;
-  segT seg_right = S_GET_SEGMENT (right);
-  const char *opname;
-
-  switch (op)
-    {
-    default:
-      abort ();
-      return;
-
-    case O_uminus:		opname = "-"; break;
-    case O_bit_not:		opname = "~"; break;
-    case O_logical_not:		opname = "!"; break;
-    case O_multiply:		opname = "*"; break;
-    case O_divide:		opname = "/"; break;
-    case O_modulus:		opname = "%"; break;
-    case O_left_shift:		opname = "<<"; break;
-    case O_right_shift:		opname = ">>"; break;
-    case O_bit_inclusive_or:	opname = "|"; break;
-    case O_bit_or_not:		opname = "|~"; break;
-    case O_bit_exclusive_or:	opname = "^"; break;
-    case O_bit_and:		opname = "&"; break;
-    case O_add:			opname = "+"; break;
-    case O_subtract:		opname = "-"; break;
-    case O_eq:			opname = "=="; break;
-    case O_ne:			opname = "!="; break;
-    case O_lt:			opname = "<"; break;
-    case O_le:			opname = "<="; break;
-    case O_ge:			opname = ">="; break;
-    case O_gt:			opname = ">"; break;
-    case O_logical_and:		opname = "&&"; break;
-    case O_logical_or:		opname = "||"; break;
-    }
+  segT seg_left = S_GET_SEGMENT (left);
+  segT seg_right = right ? S_GET_SEGMENT (right) : 0;
 
   if (expr_symbol_where (symp, &file, &line))
     {
-      if (left)
+      if (seg_left == undefined_section)
 	as_bad_where (file, line,
-		      _("invalid operands (%s and %s sections) for `%s'"),
-		      seg_left->name, seg_right->name, opname);
-      else
+		      _("undefined symbol `%s' in operation"),
+		      S_GET_NAME (left));
+      if (seg_right == undefined_section)
 	as_bad_where (file, line,
-		      _("invalid operand (%s section) for `%s'"),
-		      seg_right->name, opname);
+		      _("undefined symbol `%s' in operation"),
+		      S_GET_NAME (right));
+      if (seg_left != undefined_section
+	  && seg_right != undefined_section)
+	{
+	  if (right)
+	    as_bad_where (file, line,
+			  _("invalid sections for operation on `%s' and `%s'"),
+			  S_GET_NAME (left), S_GET_NAME (right));
+	  else
+	    as_bad_where (file, line,
+			  _("invalid section for operation on `%s'"),
+			  S_GET_NAME (left));
+	}
+
     }
   else
     {
-      const char *sname = S_GET_NAME (symp);
-
-      if (left)
-	as_bad (_("invalid operands (%s and %s sections) for `%s' when setting `%s'"),
-		seg_left->name, seg_right->name, opname, sname);
-      else
-	as_bad (_("invalid operand (%s section) for `%s' when setting `%s'"),
-		seg_right->name, opname, sname);
+      if (seg_left == undefined_section)
+	as_bad (_("undefined symbol `%s' in operation setting `%s'"),
+		S_GET_NAME (left), S_GET_NAME (symp));
+      if (seg_right == undefined_section)
+	as_bad (_("undefined symbol `%s' in operation setting `%s'"),
+		S_GET_NAME (right), S_GET_NAME (symp));
+      if (seg_left != undefined_section
+	  && seg_right != undefined_section)
+	{
+	  if (right)
+	    as_bad (_("invalid sections for operation on `%s' and `%s' setting `%s'"),
+		    S_GET_NAME (left), S_GET_NAME (right), S_GET_NAME (symp));
+	  else
+	    as_bad (_("invalid section for operation on `%s' setting `%s'"),
+		    S_GET_NAME (left), S_GET_NAME (symp));
+	}
     }
 }
 
@@ -1220,8 +1211,8 @@ resolve_symbol_value (symbolS *symp)
 		  symp->sy_value.X_add_number = final_val;
 		  /* Use X_op_symbol as a flag.  */
 		  symp->sy_value.X_op_symbol = add_symbol;
+		  final_seg = seg_left;
 		}
-	      final_seg = seg_left;
 	      final_val = 0;
 	      resolved = symbol_resolved_p (add_symbol);
 	      symp->sy_resolving = 0;
@@ -1270,7 +1261,7 @@ resolve_symbol_value (symbolS *symp)
 		~S -> S ^ ~0 	only permitted on absolute  */
 	  if (op != O_logical_not && seg_left != absolute_section
 	      && finalize_syms)
-	    report_op_error (symp, NULL, op, add_symbol);
+	    report_op_error (symp, add_symbol, NULL);
 
 	  if (final_seg == expr_section || final_seg == undefined_section)
 	    final_seg = absolute_section;
@@ -1347,7 +1338,7 @@ resolve_symbol_value (symbolS *symp)
 	     probably need to be changed for an object file format which
 	     supports arbitrary expressions, such as IEEE-695.  */
 	  if (!(seg_left == absolute_section
-		&& seg_right == absolute_section)
+		   && seg_right == absolute_section)
 	      && !(op == O_eq || op == O_ne)
 	      && !((op == O_subtract
 		    || op == O_lt || op == O_le || op == O_ge || op == O_gt)
@@ -1358,7 +1349,7 @@ resolve_symbol_value (symbolS *symp)
 	      /* Don't emit messages unless we're finalizing the symbol value,
 		 otherwise we may get the same message multiple times.  */
 	      if (finalize_syms)
-		report_op_error (symp, add_symbol, op, op_symbol);
+		report_op_error (symp, add_symbol, op_symbol);
 	      /* However do not move the symbol into the absolute section
 		 if it cannot currently be resolved - this would confuse
 		 other parts of the assembler into believing that the
-- 
1.7.4.1


[-- Attachment #3: 0002-Revert-PR-gas-12519.patch --]
[-- Type: text/x-patch, Size: 3305 bytes --]

From c51488569df7489374b5c6b2825f187186af5f8b Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@gmail.com>
Date: Tue, 8 Mar 2011 12:38:41 +0100
Subject: [PATCH 2/2] Revert "	PR gas/12519"

This reverts commit 345bbf7731af2912390e72b86807eb1b2af3e27b.
---
 gas/ChangeLog                       |    5 -----
 gas/config/obj-elf.c                |   24 ++++++++++++++++++------
 ld/testsuite/ChangeLog              |    5 -----
 ld/testsuite/ld-mn10300/i135409-3.s |    2 +-
 ld/testsuite/ld-sh/sh64/stolib.s    |    2 +-
 5 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index caa9b1e..78dc2d7 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,8 +1,3 @@
-2011-02-25  Alan Modra  <amodra@gmail.com>
-
-	PR gas/12519
-	* config/obj-elf.c (elf_frob_symbol): Properly handle size expression.
-
 2011-02-01  Alan Modra  <amodra@gmail.com>
 
 	Backport from mainline
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index 969a509..c6dc8d6 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -1,6 +1,6 @@
 /* ELF object file format
    Copyright 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000,
-   2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011
+   2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010
    Free Software Foundation, Inc.
 
    This file is part of GAS, the GNU Assembler.
@@ -1889,11 +1889,23 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 
   if (sy_obj->size != NULL)
     {
-      if (resolve_expression (sy_obj->size)
-	  && sy_obj->size->X_op == O_constant)
-	S_SET_SIZE (symp, sy_obj->size->X_add_number);
-      else
-	as_bad (_(".size expression does not evaluate to a constant"));
+      switch (sy_obj->size->X_op)
+	{
+	case O_subtract:
+	  S_SET_SIZE (symp,
+		      (S_GET_VALUE (sy_obj->size->X_add_symbol)
+		       + sy_obj->size->X_add_number
+		       - S_GET_VALUE (sy_obj->size->X_op_symbol)));
+	  break;
+	case O_constant:
+	  S_SET_SIZE (symp,
+		      (S_GET_VALUE (sy_obj->size->X_add_symbol)
+		       + sy_obj->size->X_add_number));
+	  break;
+	default:
+	  as_bad (_(".size expression too complicated to fix up"));
+	  break;
+	}
       free (sy_obj->size);
       sy_obj->size = NULL;
     }
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 1b8f84e..56a7a11 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,8 +1,3 @@
-2011-02-25  Alan Modra  <amodra@gmail.com>
-
-	* ld-mn10300/i135409-3.s: Correct .size label reference.
-	* ld-sh/sh64/stolib.s: Likewise.
-
 2011-02-10  Alan Modra  <amodra@gmail.com>
 
 	* ld-gc/start.d: Exclude frv-*-linux*.
diff --git a/ld/testsuite/ld-mn10300/i135409-3.s b/ld/testsuite/ld-mn10300/i135409-3.s
index 8113b78..e83ad96 100644
--- a/ld/testsuite/ld-mn10300/i135409-3.s
+++ b/ld/testsuite/ld-mn10300/i135409-3.s
@@ -10,7 +10,7 @@ A:
 BOTTOM:
       .balign 0x8
       add     D0,D1
-      .size   _func, .-_func
+      .size   _func, .-func
 
       .data
 L001:
diff --git a/ld/testsuite/ld-sh/sh64/stolib.s b/ld/testsuite/ld-sh/sh64/stolib.s
index a5dee2b..587faa6 100644
--- a/ld/testsuite/ld-sh/sh64/stolib.s
+++ b/ld/testsuite/ld-sh/sh64/stolib.s
@@ -4,4 +4,4 @@
 bar:
 	ptabs	r18, tr0
 	blink	tr0, r63
-.Lfe_bar: .size bar,.Lfe_bar-bar
+	.Lfe_bar: .size bar,.Lfe_bar-X
-- 
1.7.4.1

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[H.J. Lu]

On Tue, Mar 8, 2011 at 2:44 AM, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> Hi,
>
> my build of linux-next (next-20110308, the same with the one from
> yesterday) is broken.
> (I translated the German output.)
>
> [ build.log ]
>  AS      arch/x86/kernel/entry_32.o
> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
> Assembler messages:
> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
> Error: .size expression does not evaluate to a constant
> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
> make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
> make[4]: *** [arch/x86] Fehler 2 (Error 2)
> make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
> unfinished jobs...)
>

This is a kernel bug.  Please use the latest binutils from CVS.
It will tell you which symbol causes this.


-- 
H.J.

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[Sedat Dilek]

On 3/8/11, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Mar 8, 2011 at 2:44 AM, Sedat Dilek <sedat.dilek@googlemail.com>
> wrote:
>> Hi,
>>
>> my build of linux-next (next-20110308, the same with the one from
>> yesterday) is broken.
>> (I translated the German output.)
>>
>> [ build.log ]
>>  AS      arch/x86/kernel/entry_32.o
>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
>> Assembler messages:
>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
>> Error: .size expression does not evaluate to a constant
>> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
>> make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
>> make[4]: *** [arch/x86] Fehler 2 (Error 2)
>> make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
>> unfinished jobs...)
>>
>
> This is a kernel bug.  Please use the latest binutils from CVS.
> It will tell you which symbol causes this.
>
>
> --
> H.J.
>

Yeah, I have cherry-picked these two upstream commits before you have
mentionned it...

0001-Mention-symbol-name-in-non-constant-.size-expression.patch
       (Cherry-picked from commit b9521fc0be7945fc842ce1197e241a023378125d)
0002-Revert-the-last-change-on-gas-elf-bad-size.err.patch
       (Cherry-picked from commit cbd141bb69f791de7ea1581abe7afb34f0c61288)

... and have built with them a new binutils Debian package.

The error looks now like this (sorry for the German output):
...
  AS      arch/x86/kernel/entry_32.o
/home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
Assembler messages:
/home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
Error: .size expression with symbol `apf_page_fault' does not evaluate
to a constant
make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1
make[5]: *** [arch/x86/kernel] Fehler 2
make[5]: *** Warte auf noch nicht beendete Prozesse...

Anyway, before more riddling around it would be very helpful to have a
clear pointer if there is a fix around... That building, testing and
installing took me now several hours.
And... yeah, backports to 2.21-branch appreciated.

- Sedat -

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[Sedat Dilek]

[-- Attachment #1: Type: text/plain, Size: 2476 bytes --]

On 3/8/11, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> On 3/8/11, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Mar 8, 2011 at 2:44 AM, Sedat Dilek <sedat.dilek@googlemail.com>
>> wrote:
>>> Hi,
>>>
>>> my build of linux-next (next-20110308, the same with the one from
>>> yesterday) is broken.
>>> (I translated the German output.)
>>>
>>> [ build.log ]
>>>  AS      arch/x86/kernel/entry_32.o
>>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
>>> Assembler messages:
>>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
>>> Error: .size expression does not evaluate to a constant
>>> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
>>> make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
>>> make[4]: *** [arch/x86] Fehler 2 (Error 2)
>>> make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
>>> unfinished jobs...)
>>>
>>
>> This is a kernel bug.  Please use the latest binutils from CVS.
>> It will tell you which symbol causes this.
>>
>>
>> --
>> H.J.
>>
>
> Yeah, I have cherry-picked these two upstream commits before you have
> mentionned it...
>
> 0001-Mention-symbol-name-in-non-constant-.size-expression.patch
>        (Cherry-picked from commit b9521fc0be7945fc842ce1197e241a023378125d)
> 0002-Revert-the-last-change-on-gas-elf-bad-size.err.patch
>        (Cherry-picked from commit cbd141bb69f791de7ea1581abe7afb34f0c61288)
>
> ... and have built with them a new binutils Debian package.
>
> The error looks now like this (sorry for the German output):
> ...
>   AS      arch/x86/kernel/entry_32.o
> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
> Assembler messages:
> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
> Error: .size expression with symbol `apf_page_fault' does not evaluate
> to a constant
> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1
> make[5]: *** [arch/x86/kernel] Fehler 2
> make[5]: *** Warte auf noch nicht beendete Prozesse...
>
> Anyway, before more riddling around it would be very helpful to have a
> clear pointer if there is a fix around... That building, testing and
> installing took me now several hours.
> And... yeah, backports to 2.21-branch appreciated.
>
> - Sedat -
>

After a quick look into the source, it seems attached patch fixes the issue.
Is that OK?

- Sedat -

[-- Attachment #2: 0001-x86-Fix-build-failure-with-binutils-as-from-upstream.patch --]
[-- Type: text/x-patch, Size: 940 bytes --]

From c4f8070805fff7c0d097b7a0fb4f549ebd14a74b Mon Sep 17 00:00:00 2001
From: Sedat Dilek <sedat.dilek@gmail.com>
Date: Tue, 8 Mar 2011 16:32:18 +0100
Subject: [PATCH] x86: Fix build failure with binutils/as from upstream

This is from my build log:
[...]
AS      arch/x86/kernel/entry_32.o
arch/x86/kernel/entry_32.S: Assembler messages:
arch/x86/kernel/entry_32.S:1421: Error: .size expression with symbol `apf_page_fault' does not evaluate to a constant

Signed-off-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 arch/x86/kernel/entry_32.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index c8b4efa..9ca3b0e 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -1413,7 +1413,7 @@ ENTRY(async_page_fault)
 	CFI_ADJUST_CFA_OFFSET 4
 	jmp error_code
 	CFI_ENDPROC
-END(apf_page_fault)
+END(async_page_fault)
 #endif
 
 /*
-- 
1.7.4.1

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[Ingo Molnar]

* H.J. Lu <hjl.tools@gmail.com> wrote:

> On Tue, Mar 8, 2011 at 12:33 PM, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> > On Tue, Mar 8, 2011 at 9:25 PM, Alexander van Heukelum
> > <heukelum@fastmail.fm> wrote:
> >> On Tue, 08 Mar 2011 18:53 +0100, "Sedat Dilek" <sedat.dilek@googlemail.com> wrote:
> >>> On Tue, Mar 8, 2011 at 6:27 PM, Alexander van Heukelum
> >>> <heukelum@fastmail.fm> wrote:
> >>> > On Tue, 08 Mar 2011 16:42 +0100, "Sedat Dilek" <sedat.dilek@googlemail.com> wrote:
> >>> >> On 3/8/11, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> >>> >> > On 3/8/11, H.J. Lu <hjl.tools@gmail.com> wrote:
> >>> >> >> On Tue, Mar 8, 2011 at 2:44 AM, Sedat Dilek <sedat.dilek@googlemail.com>
> >>> >> >> wrote:
> >>> >> >>> Hi,
> >>> >> >>>
> >>> >> >>> my build of linux-next (next-20110308, the same with the one from
> >>> >> >>> yesterday) is broken.
> >>> >> >>> (I translated the German output.)
> >>> >> >>>
> >>> >> >>> [ build.log ]
> >>> >> >>>  AS      arch/x86/kernel/entry_32.o
> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
> >>> >> >>> Assembler messages:
> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
> >>> >> >>> Error: .size expression does not evaluate to a constant
> >>> >> >>> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
> >>> >> >>> make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
> >>> >> >>> make[4]: *** [arch/x86] Fehler 2 (Error 2)
> >>> >> >>> make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
> >>> >> >>> unfinished jobs...)
> >>> >> >>>
> >>> >> >>
> >>> >> >> This is a kernel bug.  Please use the latest binutils from CVS.
> >>> >> >> It will tell you which symbol causes this.
> >>> >> >>
> >>> >> >>
> >>> >> >> --
> >>> >> >> H.J.
> >>> >> >>
> >>> >> >
> >>> >> > Yeah, I have cherry-picked these two upstream commits before you have
> >>> >> > mentionned it...
> >>> >> >
> >>> >> > 0001-Mention-symbol-name-in-non-constant-.size-expression.patch
> >>> >> >        (Cherry-picked from commit b9521fc0be7945fc842ce1197e241a023378125d)
> >>> >> > 0002-Revert-the-last-change-on-gas-elf-bad-size.err.patch
> >>> >> >        (Cherry-picked from commit cbd141bb69f791de7ea1581abe7afb34f0c61288)
> >>> >> >
> >>> >> > ... and have built with them a new binutils Debian package.
> >>> >> >
> >>> >> > The error looks now like this (sorry for the German output):
> >>> >> > ...
> >>> >> >   AS      arch/x86/kernel/entry_32.o
> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
> >>> >> > Assembler messages:
> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
> >>> >> > Error: .size expression with symbol `apf_page_fault' does not evaluate
> >>> >> > to a constant
> >>> >> > make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1
> >>> >> > make[5]: *** [arch/x86/kernel] Fehler 2
> >>> >> > make[5]: *** Warte auf noch nicht beendete Prozesse...
> >>> >> >
> >>> >> > Anyway, before more riddling around it would be very helpful to have a
> >>> >> > clear pointer if there is a fix around... That building, testing and
> >>> >> > installing took me now several hours.
> >>> >> > And... yeah, backports to 2.21-branch appreciated.
> >>> >> >
> >>> >> > - Sedat -
> >>> >> >
> >>> >>
> >>> >> After a quick look into the source, it seems attached patch fixes the
> >>> >> issue.
> >>> >> Is that OK?
> >>> >
> >>> > Hi Sedat,
> >>> >
> >>> > The patch ( https://lkml.org/lkml/2011/3/8/203 ) is ok, feel free to add
> >>> > Acked-by: Alexander van Heukelum <heukelum@fastmail.fm>
> >>> >
> >>> > Better description might be something like:
> >>> >
> >>> > i386: Fix mismatched ENTRY/END pair.
> >>> >
> >>> > Under CONFIG_KVM_GUEST=y, the following part of entry_32.S causes a compile failure.
> >>> >
> >>> > 1409 #ifdef CONFIG_KVM_GUEST
> >>> > 1410 ENTRY(async_page_fault)
> >>> > 1411         RING0_EC_FRAME
> >>> > 1412         pushl $do_async_page_fault
> >>> > 1413         CFI_ADJUST_CFA_OFFSET 4
> >>> > 1414         jmp error_code
> >>> > 1415         CFI_ENDPROC
> >>> > 1416 END(apf_page_fault)
> >>> > 1417 #endif
> >>> >
> >>> > Replace apf_page_fault with async_page_fault, as intended.
> >>> >
> >>> > Greetings,
> >>> >    Alexander
> >>> >
> >>> >> - Sedat -
> >>> >>
> >>> >> Email had 1 attachment:
> >>> >> + 0001-x86-Fix-build-failure-with-binutils-as-from-upstream.patch
> >>> >>   1k (text/x-patch)
> >>> >
> >>>
> >>> As I said, quick view on the code, quick fix :-).
> >>>
> >>> Your description is definitive more meaningful.
> >>> I can refresh my patch and add your ACK.
> >>>
> >>> Anyway, I continued after dinner and with the above patch I ran into
> >>> the next problem:
> >>> [ build.log ]
> >>> ...
> >>>   AS      arch/x86/kernel/acpi/wakeup_rm.o
> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:
> >>> Assembler messages:
> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:12:
> >>> Error: .size expression with symbol `wakeup_code_start' does not
> >>> evaluate to a constant
> >>
> >> No idea what's wrong there. But my version of wakeup_rm.S has only 10 lines...
> >>
> >>     1  /*
> >>     2   * Wrapper script for the realmode binary as a transport object
> >>     3   * before copying to low memory.
> >>     4   */
> >>     5          .section ".rodata","a"
> >>     6          .globl  wakeup_code_start, wakeup_code_end
> >>     7  wakeup_code_start:
> >>     8          .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
> >>     9  wakeup_code_end:
> >>    10          .size   wakeup_code_start, .-wakeup_code_start
> >>
> >> And it compiles just fine.
> >> The fix for entry_32.S is valid, though, and necessary for mainline.
> >>
> >> Greetings,
> >>    Alexander
> >>
> >>> I am unsure how to fix that and open for feedback.
> >>>
> >>> - Sedat -
> >>>
> >>
> >
> > The file in linux-next (next-20110308) looks different (the above code
> > looks more logical to me)
> >
> > [ arch/x86/kernel/acpi/wakeup_rm.S ]
> >
> > /*
> >  * Wrapper script for the realmode binary as a transport object
> >  * before copying to low memory.
> >  */
> > #include <asm/page_types.h>
> >
> >        .section ".x86_trampoline","a"
> >        .balign PAGE_SIZE
> >        .globl  acpi_wakeup_code
> > acpi_wakeup_code:
> >        .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
> >        .size   wakeup_code_start, .-wakeup_code_start
> >
> 
> Those are simply wrong.  2.6.38-rc8 is OK.

2.6.37-rc8 is not OK: for example commit 631bc4878220932fe67fc46fc7cf7cccdb1ec597 is 
already upstream and if you enable KVM you see a broken kernel build with new 
binutils. This is from 2.6.38-rc8:

 #ifdef CONFIG_KVM_GUEST
 ENTRY(async_page_fault)
        RING0_EC_FRAME
        pushl $do_async_page_fault
        CFI_ADJUST_CFA_OFFSET 4
        jmp error_code
        CFI_ENDPROC
 END(apf_page_fault)
 #endif

Yes, the .size directive not matching up is technically a bug, but it was not 
checked by binutils before, for *years* - and you cannot just flip a switch and 
break who knows how much code.

You need to at least emit a warning for some time to give people a *chance* to fix 
bugs - not just stuff an incompatible binutils down their throat and break the 
kernel build for thousands of commits and break bisection.

This binutils change is breaking numerous upstream kernel builds (and is making 
bisection with new binutils impossible) for no particular good reason: binutils was 
capable to figure out the symbol name before this change.

At minimum you need to *understand* that what you are doing is an incompatible 
change and is disruptive to others ...

Thanks,

	Ingo

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[Sedat Dilek]

On Wed, Mar 9, 2011 at 9:51 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * H.J. Lu <hjl.tools@gmail.com> wrote:
>
>> On Tue, Mar 8, 2011 at 12:33 PM, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
>> > On Tue, Mar 8, 2011 at 9:25 PM, Alexander van Heukelum
>> > <heukelum@fastmail.fm> wrote:
>> >> On Tue, 08 Mar 2011 18:53 +0100, "Sedat Dilek" <sedat.dilek@googlemail.com> wrote:
>> >>> On Tue, Mar 8, 2011 at 6:27 PM, Alexander van Heukelum
>> >>> <heukelum@fastmail.fm> wrote:
>> >>> > On Tue, 08 Mar 2011 16:42 +0100, "Sedat Dilek" <sedat.dilek@googlemail.com> wrote:
>> >>> >> On 3/8/11, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
>> >>> >> > On 3/8/11, H.J. Lu <hjl.tools@gmail.com> wrote:
>> >>> >> >> On Tue, Mar 8, 2011 at 2:44 AM, Sedat Dilek <sedat.dilek@googlemail.com>
>> >>> >> >> wrote:
>> >>> >> >>> Hi,
>> >>> >> >>>
>> >>> >> >>> my build of linux-next (next-20110308, the same with the one from
>> >>> >> >>> yesterday) is broken.
>> >>> >> >>> (I translated the German output.)
>> >>> >> >>>
>> >>> >> >>> [ build.log ]
>> >>> >> >>>  AS      arch/x86/kernel/entry_32.o
>> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
>> >>> >> >>> Assembler messages:
>> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
>> >>> >> >>> Error: .size expression does not evaluate to a constant
>> >>> >> >>> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
>> >>> >> >>> make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
>> >>> >> >>> make[4]: *** [arch/x86] Fehler 2 (Error 2)
>> >>> >> >>> make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
>> >>> >> >>> unfinished jobs...)
>> >>> >> >>>
>> >>> >> >>
>> >>> >> >> This is a kernel bug.  Please use the latest binutils from CVS.
>> >>> >> >> It will tell you which symbol causes this.
>> >>> >> >>
>> >>> >> >>
>> >>> >> >> --
>> >>> >> >> H.J.
>> >>> >> >>
>> >>> >> >
>> >>> >> > Yeah, I have cherry-picked these two upstream commits before you have
>> >>> >> > mentionned it...
>> >>> >> >
>> >>> >> > 0001-Mention-symbol-name-in-non-constant-.size-expression.patch
>> >>> >> >        (Cherry-picked from commit b9521fc0be7945fc842ce1197e241a023378125d)
>> >>> >> > 0002-Revert-the-last-change-on-gas-elf-bad-size.err.patch
>> >>> >> >        (Cherry-picked from commit cbd141bb69f791de7ea1581abe7afb34f0c61288)
>> >>> >> >
>> >>> >> > ... and have built with them a new binutils Debian package.
>> >>> >> >
>> >>> >> > The error looks now like this (sorry for the German output):
>> >>> >> > ...
>> >>> >> >   AS      arch/x86/kernel/entry_32.o
>> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
>> >>> >> > Assembler messages:
>> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
>> >>> >> > Error: .size expression with symbol `apf_page_fault' does not evaluate
>> >>> >> > to a constant
>> >>> >> > make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1
>> >>> >> > make[5]: *** [arch/x86/kernel] Fehler 2
>> >>> >> > make[5]: *** Warte auf noch nicht beendete Prozesse...
>> >>> >> >
>> >>> >> > Anyway, before more riddling around it would be very helpful to have a
>> >>> >> > clear pointer if there is a fix around... That building, testing and
>> >>> >> > installing took me now several hours.
>> >>> >> > And... yeah, backports to 2.21-branch appreciated.
>> >>> >> >
>> >>> >> > - Sedat -
>> >>> >> >
>> >>> >>
>> >>> >> After a quick look into the source, it seems attached patch fixes the
>> >>> >> issue.
>> >>> >> Is that OK?
>> >>> >
>> >>> > Hi Sedat,
>> >>> >
>> >>> > The patch ( https://lkml.org/lkml/2011/3/8/203 ) is ok, feel free to add
>> >>> > Acked-by: Alexander van Heukelum <heukelum@fastmail.fm>
>> >>> >
>> >>> > Better description might be something like:
>> >>> >
>> >>> > i386: Fix mismatched ENTRY/END pair.
>> >>> >
>> >>> > Under CONFIG_KVM_GUEST=y, the following part of entry_32.S causes a compile failure.
>> >>> >
>> >>> > 1409 #ifdef CONFIG_KVM_GUEST
>> >>> > 1410 ENTRY(async_page_fault)
>> >>> > 1411         RING0_EC_FRAME
>> >>> > 1412         pushl $do_async_page_fault
>> >>> > 1413         CFI_ADJUST_CFA_OFFSET 4
>> >>> > 1414         jmp error_code
>> >>> > 1415         CFI_ENDPROC
>> >>> > 1416 END(apf_page_fault)
>> >>> > 1417 #endif
>> >>> >
>> >>> > Replace apf_page_fault with async_page_fault, as intended.
>> >>> >
>> >>> > Greetings,
>> >>> >    Alexander
>> >>> >
>> >>> >> - Sedat -
>> >>> >>
>> >>> >> Email had 1 attachment:
>> >>> >> + 0001-x86-Fix-build-failure-with-binutils-as-from-upstream.patch
>> >>> >>   1k (text/x-patch)
>> >>> >
>> >>>
>> >>> As I said, quick view on the code, quick fix :-).
>> >>>
>> >>> Your description is definitive more meaningful.
>> >>> I can refresh my patch and add your ACK.
>> >>>
>> >>> Anyway, I continued after dinner and with the above patch I ran into
>> >>> the next problem:
>> >>> [ build.log ]
>> >>> ...
>> >>>   AS      arch/x86/kernel/acpi/wakeup_rm.o
>> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:
>> >>> Assembler messages:
>> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:12:
>> >>> Error: .size expression with symbol `wakeup_code_start' does not
>> >>> evaluate to a constant
>> >>
>> >> No idea what's wrong there. But my version of wakeup_rm.S has only 10 lines...
>> >>
>> >>     1  /*
>> >>     2   * Wrapper script for the realmode binary as a transport object
>> >>     3   * before copying to low memory.
>> >>     4   */
>> >>     5          .section ".rodata","a"
>> >>     6          .globl  wakeup_code_start, wakeup_code_end
>> >>     7  wakeup_code_start:
>> >>     8          .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
>> >>     9  wakeup_code_end:
>> >>    10          .size   wakeup_code_start, .-wakeup_code_start
>> >>
>> >> And it compiles just fine.
>> >> The fix for entry_32.S is valid, though, and necessary for mainline.
>> >>
>> >> Greetings,
>> >>    Alexander
>> >>
>> >>> I am unsure how to fix that and open for feedback.
>> >>>
>> >>> - Sedat -
>> >>>
>> >>
>> >
>> > The file in linux-next (next-20110308) looks different (the above code
>> > looks more logical to me)
>> >
>> > [ arch/x86/kernel/acpi/wakeup_rm.S ]
>> >
>> > /*
>> >  * Wrapper script for the realmode binary as a transport object
>> >  * before copying to low memory.
>> >  */
>> > #include <asm/page_types.h>
>> >
>> >        .section ".x86_trampoline","a"
>> >        .balign PAGE_SIZE
>> >        .globl  acpi_wakeup_code
>> > acpi_wakeup_code:
>> >        .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
>> >        .size   wakeup_code_start, .-wakeup_code_start
>> >
>>
>> Those are simply wrong.  2.6.38-rc8 is OK.
>
> 2.6.37-rc8 is not OK: for example commit 631bc4878220932fe67fc46fc7cf7cccdb1ec597 is
> already upstream and if you enable KVM you see a broken kernel build with new
> binutils. This is from 2.6.38-rc8:
>
>  #ifdef CONFIG_KVM_GUEST
>  ENTRY(async_page_fault)
>        RING0_EC_FRAME
>        pushl $do_async_page_fault
>        CFI_ADJUST_CFA_OFFSET 4
>        jmp error_code
>        CFI_ENDPROC
>  END(apf_page_fault)
>  #endif
>
> Yes, the .size directive not matching up is technically a bug, but it was not
> checked by binutils before, for *years* - and you cannot just flip a switch and
> break who knows how much code.
>
> You need to at least emit a warning for some time to give people a *chance* to fix
> bugs - not just stuff an incompatible binutils down their throat and break the
> kernel build for thousands of commits and break bisection.
>
> This binutils change is breaking numerous upstream kernel builds (and is making
> bisection with new binutils impossible) for no particular good reason: binutils was
> capable to figure out the symbol name before this change.
>
> At minimum you need to *understand* that what you are doing is an incompatible
> change and is disruptive to others ...
>
> Thanks,
>
>        Ingo
>

OK, I am not a toolchain expert, but this recent changes to upstream
binutils were helpful to find at least the problematic place in the
code (fix see [1]).
I only needed a blink of an eye to catch it.
binutils 2.21 GIT has this change "PR gas/12519" which was not "perfect".
With the patch in upstream (see [2], which should be cherry-picked for
2.21-GIT as I did for my debianized binutils), there is more
"verbosity", now.
We have now an advantage as we know what's going on *before* final
binutils-2.21 release.

What are you expecting from binutils developers?
Shall they revert the above PR (yes, I also build a binutils with a
revert of it)?

The next place in code (I am on linux-next) could also be found easily
and with some help of x86 folks it could be fixed [2], too.

Can you please explain the "incompatibility" and "breakage" with older
kernels (especially for bisecting sounds awful)?
(I am just curious and the several builds of binutils and linux-next
kernels I did yesterday should not be for nothing :-).)

If I can help (like testing or whatever), please let me know.

- Sedat -

[1] https://patchwork.kernel.org/patch/619081/
[2] http://sourceware.org/ml/binutils/2011-03/msg00078.html
[3] https://patchwork.kernel.org/patch/619611/

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[Ingo Molnar]

* Sedat Dilek <sedat.dilek@googlemail.com> wrote:

> On Wed, Mar 9, 2011 at 9:51 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> >> On Tue, Mar 8, 2011 at 12:33 PM, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> >> > On Tue, Mar 8, 2011 at 9:25 PM, Alexander van Heukelum
> >> > <heukelum@fastmail.fm> wrote:
> >> >> On Tue, 08 Mar 2011 18:53 +0100, "Sedat Dilek" <sedat.dilek@googlemail.com> wrote:
> >> >>> On Tue, Mar 8, 2011 at 6:27 PM, Alexander van Heukelum
> >> >>> <heukelum@fastmail.fm> wrote:
> >> >>> > On Tue, 08 Mar 2011 16:42 +0100, "Sedat Dilek" <sedat.dilek@googlemail.com> wrote:
> >> >>> >> On 3/8/11, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> >> >>> >> > On 3/8/11, H.J. Lu <hjl.tools@gmail.com> wrote:
> >> >>> >> >> On Tue, Mar 8, 2011 at 2:44 AM, Sedat Dilek <sedat.dilek@googlemail.com>
> >> >>> >> >> wrote:
> >> >>> >> >>> Hi,
> >> >>> >> >>>
> >> >>> >> >>> my build of linux-next (next-20110308, the same with the one from
> >> >>> >> >>> yesterday) is broken.
> >> >>> >> >>> (I translated the German output.)
> >> >>> >> >>>
> >> >>> >> >>> [ build.log ]
> >> >>> >> >>>  AS      arch/x86/kernel/entry_32.o
> >> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
> >> >>> >> >>> Assembler messages:
> >> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
> >> >>> >> >>> Error: .size expression does not evaluate to a constant
> >> >>> >> >>> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
> >> >>> >> >>> make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
> >> >>> >> >>> make[4]: *** [arch/x86] Fehler 2 (Error 2)
> >> >>> >> >>> make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
> >> >>> >> >>> unfinished jobs...)
> >> >>> >> >>>
> >> >>> >> >>
> >> >>> >> >> This is a kernel bug.  Please use the latest binutils from CVS.
> >> >>> >> >> It will tell you which symbol causes this.
> >> >>> >> >>
> >> >>> >> >>
> >> >>> >> >> --
> >> >>> >> >> H.J.
> >> >>> >> >>
> >> >>> >> >
> >> >>> >> > Yeah, I have cherry-picked these two upstream commits before you have
> >> >>> >> > mentionned it...
> >> >>> >> >
> >> >>> >> > 0001-Mention-symbol-name-in-non-constant-.size-expression.patch
> >> >>> >> >        (Cherry-picked from commit b9521fc0be7945fc842ce1197e241a023378125d)
> >> >>> >> > 0002-Revert-the-last-change-on-gas-elf-bad-size.err.patch
> >> >>> >> >        (Cherry-picked from commit cbd141bb69f791de7ea1581abe7afb34f0c61288)
> >> >>> >> >
> >> >>> >> > ... and have built with them a new binutils Debian package.
> >> >>> >> >
> >> >>> >> > The error looks now like this (sorry for the German output):
> >> >>> >> > ...
> >> >>> >> >   AS      arch/x86/kernel/entry_32.o
> >> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
> >> >>> >> > Assembler messages:
> >> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
> >> >>> >> > Error: .size expression with symbol `apf_page_fault' does not evaluate
> >> >>> >> > to a constant
> >> >>> >> > make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1
> >> >>> >> > make[5]: *** [arch/x86/kernel] Fehler 2
> >> >>> >> > make[5]: *** Warte auf noch nicht beendete Prozesse...
> >> >>> >> >
> >> >>> >> > Anyway, before more riddling around it would be very helpful to have a
> >> >>> >> > clear pointer if there is a fix around... That building, testing and
> >> >>> >> > installing took me now several hours.
> >> >>> >> > And... yeah, backports to 2.21-branch appreciated.
> >> >>> >> >
> >> >>> >> > - Sedat -
> >> >>> >> >
> >> >>> >>
> >> >>> >> After a quick look into the source, it seems attached patch fixes the
> >> >>> >> issue.
> >> >>> >> Is that OK?
> >> >>> >
> >> >>> > Hi Sedat,
> >> >>> >
> >> >>> > The patch ( https://lkml.org/lkml/2011/3/8/203 ) is ok, feel free to add
> >> >>> > Acked-by: Alexander van Heukelum <heukelum@fastmail.fm>
> >> >>> >
> >> >>> > Better description might be something like:
> >> >>> >
> >> >>> > i386: Fix mismatched ENTRY/END pair.
> >> >>> >
> >> >>> > Under CONFIG_KVM_GUEST=y, the following part of entry_32.S causes a compile failure.
> >> >>> >
> >> >>> > 1409 #ifdef CONFIG_KVM_GUEST
> >> >>> > 1410 ENTRY(async_page_fault)
> >> >>> > 1411         RING0_EC_FRAME
> >> >>> > 1412         pushl $do_async_page_fault
> >> >>> > 1413         CFI_ADJUST_CFA_OFFSET 4
> >> >>> > 1414         jmp error_code
> >> >>> > 1415         CFI_ENDPROC
> >> >>> > 1416 END(apf_page_fault)
> >> >>> > 1417 #endif
> >> >>> >
> >> >>> > Replace apf_page_fault with async_page_fault, as intended.
> >> >>> >
> >> >>> > Greetings,
> >> >>> >    Alexander
> >> >>> >
> >> >>> >> - Sedat -
> >> >>> >>
> >> >>> >> Email had 1 attachment:
> >> >>> >> + 0001-x86-Fix-build-failure-with-binutils-as-from-upstream.patch
> >> >>> >>   1k (text/x-patch)
> >> >>> >
> >> >>>
> >> >>> As I said, quick view on the code, quick fix :-).
> >> >>>
> >> >>> Your description is definitive more meaningful.
> >> >>> I can refresh my patch and add your ACK.
> >> >>>
> >> >>> Anyway, I continued after dinner and with the above patch I ran into
> >> >>> the next problem:
> >> >>> [ build.log ]
> >> >>> ...
> >> >>>   AS      arch/x86/kernel/acpi/wakeup_rm.o
> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:
> >> >>> Assembler messages:
> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:12:
> >> >>> Error: .size expression with symbol `wakeup_code_start' does not
> >> >>> evaluate to a constant
> >> >>
> >> >> No idea what's wrong there. But my version of wakeup_rm.S has only 10 lines...
> >> >>
> >> >>     1  /*
> >> >>     2   * Wrapper script for the realmode binary as a transport object
> >> >>     3   * before copying to low memory.
> >> >>     4   */
> >> >>     5          .section ".rodata","a"
> >> >>     6          .globl  wakeup_code_start, wakeup_code_end
> >> >>     7  wakeup_code_start:
> >> >>     8          .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
> >> >>     9  wakeup_code_end:
> >> >>    10          .size   wakeup_code_start, .-wakeup_code_start
> >> >>
> >> >> And it compiles just fine.
> >> >> The fix for entry_32.S is valid, though, and necessary for mainline.
> >> >>
> >> >> Greetings,
> >> >>    Alexander
> >> >>
> >> >>> I am unsure how to fix that and open for feedback.
> >> >>>
> >> >>> - Sedat -
> >> >>>
> >> >>
> >> >
> >> > The file in linux-next (next-20110308) looks different (the above code
> >> > looks more logical to me)
> >> >
> >> > [ arch/x86/kernel/acpi/wakeup_rm.S ]
> >> >
> >> > /*
> >> >  * Wrapper script for the realmode binary as a transport object
> >> >  * before copying to low memory.
> >> >  */
> >> > #include <asm/page_types.h>
> >> >
> >> >        .section ".x86_trampoline","a"
> >> >        .balign PAGE_SIZE
> >> >        .globl  acpi_wakeup_code
> >> > acpi_wakeup_code:
> >> >        .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
> >> >        .size   wakeup_code_start, .-wakeup_code_start
> >> >
> >>
> >> Those are simply wrong.  2.6.38-rc8 is OK.
> >
> > 2.6.37-rc8 is not OK: for example commit 631bc4878220932fe67fc46fc7cf7cccdb1ec597 is
> > already upstream and if you enable KVM you see a broken kernel build with new
> > binutils. This is from 2.6.38-rc8:
> >
> >  #ifdef CONFIG_KVM_GUEST
> >  ENTRY(async_page_fault)
> >        RING0_EC_FRAME
> >        pushl $do_async_page_fault
> >        CFI_ADJUST_CFA_OFFSET 4
> >        jmp error_code
> >        CFI_ENDPROC
> >  END(apf_page_fault)
> >  #endif
> >
> > Yes, the .size directive not matching up is technically a bug, but it was not
> > checked by binutils before, for *years* - and you cannot just flip a switch and
> > break who knows how much code.
> >
> > You need to at least emit a warning for some time to give people a *chance* to fix
> > bugs - not just stuff an incompatible binutils down their throat and break the
> > kernel build for thousands of commits and break bisection.
> >
> > This binutils change is breaking numerous upstream kernel builds (and is making
> > bisection with new binutils impossible) for no particular good reason: binutils was
> > capable to figure out the symbol name before this change.
> >
> > At minimum you need to *understand* that what you are doing is an incompatible
> > change and is disruptive to others ...
> >
> > Thanks,
> >
> >        Ingo
> >
> 
> OK, I am not a toolchain expert, but this recent changes to upstream
> binutils were helpful to find at least the problematic place in the
> code (fix see [1]).

A warning would have a similarly positive effect.

> I only needed a blink of an eye to catch it.
> binutils 2.21 GIT has this change "PR gas/12519" which was not "perfect".
> With the patch in upstream (see [2], which should be cherry-picked for
> 2.21-GIT as I did for my debianized binutils), there is more
> "verbosity", now.
> We have now an advantage as we know what's going on *before* final
> binutils-2.21 release.
> 
> What are you expecting from binutils developers?
> Shall they revert the above PR (yes, I also build a binutils with a
> revert of it)?
> 
> The next place in code (I am on linux-next) could also be found easily
> and with some help of x86 folks it could be fixed [2], too.
> 
> Can you please explain the "incompatibility" and "breakage" with older
> kernels (especially for bisecting sounds awful)?

The KVM commit that had the mismatched pair is upstream:

 631bc4878220: KVM: Handle async PF in a guest.

You need to have CONFIG_KVM_GUEST=y enabled in your .config to hit that build 
failure. If in the future, on a binutils-2.21 system you end up bisecting into this 
commit or into any commit that contains that commit but not the fix, the build 
breaks.

Also, while such mismatches get found eventually, they never had any bad 
side-effects, so they were only found and fixed based on review.

Thus there's an unknown number of commits in the Linux kernel history that wont 
build with binutils-2.21, with an unknown combination of .config's.

It might not matter much, but you should be aware of it - and unless it's absolutely 
vital to change binutils behavior here it would be well advised to at least consider 
emitting a warning first and a hard build failure in a later version - that would 
soften most of the direct practical impact of such a change (most bisection targets 
involve the previous 1-2 kernel releases).

Thanks,

	Ingo

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[H.J. Lu]

On Wed, Mar 9, 2011 at 12:51 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * H.J. Lu <hjl.tools@gmail.com> wrote:
>
>> On Tue, Mar 8, 2011 at 12:33 PM, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
>> > On Tue, Mar 8, 2011 at 9:25 PM, Alexander van Heukelum
>> > <heukelum@fastmail.fm> wrote:
>> >> On Tue, 08 Mar 2011 18:53 +0100, "Sedat Dilek" <sedat.dilek@googlemail.com> wrote:
>> >>> On Tue, Mar 8, 2011 at 6:27 PM, Alexander van Heukelum
>> >>> <heukelum@fastmail.fm> wrote:
>> >>> > On Tue, 08 Mar 2011 16:42 +0100, "Sedat Dilek" <sedat.dilek@googlemail.com> wrote:
>> >>> >> On 3/8/11, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
>> >>> >> > On 3/8/11, H.J. Lu <hjl.tools@gmail.com> wrote:
>> >>> >> >> On Tue, Mar 8, 2011 at 2:44 AM, Sedat Dilek <sedat.dilek@googlemail.com>
>> >>> >> >> wrote:
>> >>> >> >>> Hi,
>> >>> >> >>>
>> >>> >> >>> my build of linux-next (next-20110308, the same with the one from
>> >>> >> >>> yesterday) is broken.
>> >>> >> >>> (I translated the German output.)
>> >>> >> >>>
>> >>> >> >>> [ build.log ]
>> >>> >> >>>  AS      arch/x86/kernel/entry_32.o
>> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
>> >>> >> >>> Assembler messages:
>> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
>> >>> >> >>> Error: .size expression does not evaluate to a constant
>> >>> >> >>> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
>> >>> >> >>> make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
>> >>> >> >>> make[4]: *** [arch/x86] Fehler 2 (Error 2)
>> >>> >> >>> make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
>> >>> >> >>> unfinished jobs...)
>> >>> >> >>>
>> >>> >> >>
>> >>> >> >> This is a kernel bug.  Please use the latest binutils from CVS.
>> >>> >> >> It will tell you which symbol causes this.
>> >>> >> >>
>> >>> >> >>
>> >>> >> >> --
>> >>> >> >> H.J.
>> >>> >> >>
>> >>> >> >
>> >>> >> > Yeah, I have cherry-picked these two upstream commits before you have
>> >>> >> > mentionned it...
>> >>> >> >
>> >>> >> > 0001-Mention-symbol-name-in-non-constant-.size-expression.patch
>> >>> >> >        (Cherry-picked from commit b9521fc0be7945fc842ce1197e241a023378125d)
>> >>> >> > 0002-Revert-the-last-change-on-gas-elf-bad-size.err.patch
>> >>> >> >        (Cherry-picked from commit cbd141bb69f791de7ea1581abe7afb34f0c61288)
>> >>> >> >
>> >>> >> > ... and have built with them a new binutils Debian package.
>> >>> >> >
>> >>> >> > The error looks now like this (sorry for the German output):
>> >>> >> > ...
>> >>> >> >   AS      arch/x86/kernel/entry_32.o
>> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
>> >>> >> > Assembler messages:
>> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
>> >>> >> > Error: .size expression with symbol `apf_page_fault' does not evaluate
>> >>> >> > to a constant
>> >>> >> > make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1
>> >>> >> > make[5]: *** [arch/x86/kernel] Fehler 2
>> >>> >> > make[5]: *** Warte auf noch nicht beendete Prozesse...
>> >>> >> >
>> >>> >> > Anyway, before more riddling around it would be very helpful to have a
>> >>> >> > clear pointer if there is a fix around... That building, testing and
>> >>> >> > installing took me now several hours.
>> >>> >> > And... yeah, backports to 2.21-branch appreciated.
>> >>> >> >
>> >>> >> > - Sedat -
>> >>> >> >
>> >>> >>
>> >>> >> After a quick look into the source, it seems attached patch fixes the
>> >>> >> issue.
>> >>> >> Is that OK?
>> >>> >
>> >>> > Hi Sedat,
>> >>> >
>> >>> > The patch ( https://lkml.org/lkml/2011/3/8/203 ) is ok, feel free to add
>> >>> > Acked-by: Alexander van Heukelum <heukelum@fastmail.fm>
>> >>> >
>> >>> > Better description might be something like:
>> >>> >
>> >>> > i386: Fix mismatched ENTRY/END pair.
>> >>> >
>> >>> > Under CONFIG_KVM_GUEST=y, the following part of entry_32.S causes a compile failure.
>> >>> >
>> >>> > 1409 #ifdef CONFIG_KVM_GUEST
>> >>> > 1410 ENTRY(async_page_fault)
>> >>> > 1411         RING0_EC_FRAME
>> >>> > 1412         pushl $do_async_page_fault
>> >>> > 1413         CFI_ADJUST_CFA_OFFSET 4
>> >>> > 1414         jmp error_code
>> >>> > 1415         CFI_ENDPROC
>> >>> > 1416 END(apf_page_fault)
>> >>> > 1417 #endif
>> >>> >
>> >>> > Replace apf_page_fault with async_page_fault, as intended.
>> >>> >
>> >>> > Greetings,
>> >>> >    Alexander
>> >>> >
>> >>> >> - Sedat -
>> >>> >>
>> >>> >> Email had 1 attachment:
>> >>> >> + 0001-x86-Fix-build-failure-with-binutils-as-from-upstream.patch
>> >>> >>   1k (text/x-patch)
>> >>> >
>> >>>
>> >>> As I said, quick view on the code, quick fix :-).
>> >>>
>> >>> Your description is definitive more meaningful.
>> >>> I can refresh my patch and add your ACK.
>> >>>
>> >>> Anyway, I continued after dinner and with the above patch I ran into
>> >>> the next problem:
>> >>> [ build.log ]
>> >>> ...
>> >>>   AS      arch/x86/kernel/acpi/wakeup_rm.o
>> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:
>> >>> Assembler messages:
>> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:12:
>> >>> Error: .size expression with symbol `wakeup_code_start' does not
>> >>> evaluate to a constant
>> >>
>> >> No idea what's wrong there. But my version of wakeup_rm.S has only 10 lines...
>> >>
>> >>     1  /*
>> >>     2   * Wrapper script for the realmode binary as a transport object
>> >>     3   * before copying to low memory.
>> >>     4   */
>> >>     5          .section ".rodata","a"
>> >>     6          .globl  wakeup_code_start, wakeup_code_end
>> >>     7  wakeup_code_start:
>> >>     8          .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
>> >>     9  wakeup_code_end:
>> >>    10          .size   wakeup_code_start, .-wakeup_code_start
>> >>
>> >> And it compiles just fine.
>> >> The fix for entry_32.S is valid, though, and necessary for mainline.
>> >>
>> >> Greetings,
>> >>    Alexander
>> >>
>> >>> I am unsure how to fix that and open for feedback.
>> >>>
>> >>> - Sedat -
>> >>>
>> >>
>> >
>> > The file in linux-next (next-20110308) looks different (the above code
>> > looks more logical to me)
>> >
>> > [ arch/x86/kernel/acpi/wakeup_rm.S ]
>> >
>> > /*
>> >  * Wrapper script for the realmode binary as a transport object
>> >  * before copying to low memory.
>> >  */
>> > #include <asm/page_types.h>
>> >
>> >        .section ".x86_trampoline","a"
>> >        .balign PAGE_SIZE
>> >        .globl  acpi_wakeup_code
>> > acpi_wakeup_code:
>> >        .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
>> >        .size   wakeup_code_start, .-wakeup_code_start
>> >
>>
>> Those are simply wrong.  2.6.38-rc8 is OK.
>
> 2.6.37-rc8 is not OK: for example commit 631bc4878220932fe67fc46fc7cf7cccdb1ec597 is
> already upstream and if you enable KVM you see a broken kernel build with new
> binutils. This is from 2.6.38-rc8:
>
>  #ifdef CONFIG_KVM_GUEST
>  ENTRY(async_page_fault)
>        RING0_EC_FRAME
>        pushl $do_async_page_fault
>        CFI_ADJUST_CFA_OFFSET 4
>        jmp error_code
>        CFI_ENDPROC
>  END(apf_page_fault)
>  #endif
>
> Yes, the .size directive not matching up is technically a bug, but it was not
> checked by binutils before, for *years* - and you cannot just flip a switch and
> break who knows how much code.
>
> You need to at least emit a warning for some time to give people a *chance* to fix
> bugs - not just stuff an incompatible binutils down their throat and break the
> kernel build for thousands of commits and break bisection.

If kernel doesn't use/need symbol size, don't put it in assembly code.

> This binutils change is breaking numerous upstream kernel builds (and is making
> bisection with new binutils impossible) for no particular good reason: binutils was
> capable to figure out the symbol name before this change.
>

That is totally false. The old assembler just generates incorrect size and
It couldn't read the programmer's mind to find the correct symbol name.


-- 
H.J.

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[Ingo Molnar]

* H.J. Lu <hjl.tools@gmail.com> wrote:

> > Yes, the .size directive not matching up is technically a bug, but it was not 
> > checked by binutils before, for *years* - and you cannot just flip a switch and 
> > break who knows how much code.
> >
> > You need to at least emit a warning for some time to give people a *chance* to 
> > fix bugs - not just stuff an incompatible binutils down their throat and break 
> > the kernel build for thousands of commits and break bisection.
> 
> If kernel doesn't use/need symbol size, don't put it in assembly code.

What does that have to do with the question i raised: that of binutils backwards 
(in)compatibility to be able to build the Linux kernel?

> > This binutils change is breaking numerous upstream kernel builds (and is making 
> > bisection with new binutils impossible) for no particular good reason: binutils 
> > was capable to figure out the symbol name before this change.
> 
> That is totally false. The old assembler just generates incorrect size and It 
> couldn't read the programmer's mind to find the correct symbol name.

Still it didnt make the kernel not build and it did not make the kernel not boot. It 
at most confused some ELF symbol table - which the kernel does not need.

So all i'm trying to point out to you is that you appear to be seeing the world in a 
very binary way: 'broken' versus 'correct' and that it can be rather harmful if you 
project that binary view on the real world that in reality is not binary but is 
shades of grey.

You are turning a benign ELF symbol table detail (that does not affect the kernel's 
ability to boot/work) into a hard build breakage and bisection barrier, covering 
hundreds of commits.

I'm not denying that it's buggy code - I'm just asking you to *PLEASE* at minimum 
acknowledge that surprise flag days that turn a before-benign condition into a fatal 
build failure suck to everyone else outside your own little universe.

Thanks,

	Ingo

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[H.J. Lu]

On Wed, Mar 9, 2011 at 8:02 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
>> > This binutils change is breaking numerous upstream kernel builds (and is making
>> > bisection with new binutils impossible) for no particular good reason: binutils
>> > was capable to figure out the symbol name before this change.
>>
>> That is totally false. The old assembler just generates incorrect size and It
>> couldn't read the programmer's mind to find the correct symbol name.
>
> Still it didnt make the kernel not build and it did not make the kernel not boot. It
> at most confused some ELF symbol table - which the kernel does not need.
>
> So all i'm trying to point out to you is that you appear to be seeing the world in a
> very binary way: 'broken' versus 'correct' and that it can be rather harmful if you
> project that binary view on the real world that in reality is not binary but is
> shades of grey.
>
> You are turning a benign ELF symbol table detail (that does not affect the kernel's
> ability to boot/work) into a hard build breakage and bisection barrier, covering
> hundreds of commits.

The original assembler change didn't mention the bogus symbol name
in error message, which is very unhelpful. I changed the assembler to
print out the bogus symbol name in error message. It should be very
trivial to identify the bogus kernel assembly codes.

> I'm not denying that it's buggy code - I'm just asking you to *PLEASE* at minimum
> acknowledge that surprise flag days that turn a before-benign condition into a fatal
> build failure suck to everyone else outside your own little universe.
>

There is no way for assembler to know if a bug in input source code is
"benign" or
not. For assembler, a bug in input is a bug in input.  Assembly programmers
should appreciate this assembler change for helping him/her catch such bugs
in his/her codes.


-- 
H.J.

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[Ingo Molnar]

* H.J. Lu <hjl.tools@gmail.com> wrote:

> On Wed, Mar 9, 2011 at 8:02 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> >> > This binutils change is breaking numerous upstream kernel builds (and is making
> >> > bisection with new binutils impossible) for no particular good reason: binutils
> >> > was capable to figure out the symbol name before this change.
> >>
> >> That is totally false. The old assembler just generates incorrect size and It
> >> couldn't read the programmer's mind to find the correct symbol name.
> >
> > Still it didnt make the kernel not build and it did not make the kernel not boot. It
> > at most confused some ELF symbol table - which the kernel does not need.
> >
> > So all i'm trying to point out to you is that you appear to be seeing the world in a
> > very binary way: 'broken' versus 'correct' and that it can be rather harmful if you
> > project that binary view on the real world that in reality is not binary but is
> > shades of grey.
> >
> > You are turning a benign ELF symbol table detail (that does not affect the kernel's
> > ability to boot/work) into a hard build breakage and bisection barrier, covering
> > hundreds of commits.
> 
> The original assembler change didn't mention the bogus symbol name in error 
> message, which is very unhelpful. I changed the assembler to print out the bogus 
> symbol name in error message. It should be very trivial to identify the bogus 
> kernel assembly codes.
> 
> > I'm not denying that it's buggy code - I'm just asking you to *PLEASE* at 
> > minimum acknowledge that surprise flag days that turn a before-benign condition 
> > into a fatal build failure suck to everyone else outside your own little 
> > universe.
> 
> There is no way for assembler to know if a bug in input source code is "benign" or 
> not. For assembler, a bug in input is a bug in input.  Assembly programmers should 
> appreciate this assembler change for helping him/her catch such bugs in his/her 
> codes.

I think you _still_ have not understood the very simple compatibility point i'm 
trying to make.

You are retroactively build-breaking kernel code that built and worked fine before. 
Your change could turn large sections of kernel history into a build-broken mess.

GCC does this more or less correctly: i can still build very old versions of the 
kernel even with latest GCC - i have recently bisected to and build v2.6.20 which is 
now a 4+ years old kernel, and had no problems building and booting such an old 
kernel, with a new GCC version.

GCC does this by trying to introduce new restrictions on successful compilation 
conservatively: if the compiler gets smarter and notices bugs (or potential bugs) it 
does not abort the build, it emits a warning instead.

Your "it's simple to fix" argument misses that point: i was talking about bisection 
breakage, and during bisection of kernel bugs we dive back into older versions of 
the code.

It does not matter that you declare the .size directive bogus. It is buggy and yes 
i've already queued up the fixes and i apprecitate them being fixed, but that's 
immaterial to the compatibility argument i made: BINUTILS BUILT THE KERNEL FINE 
BEFORE.

If you do changes to a major component of the build environment of thousands of free 
software projects you *must* consider the real-life compatbility effects of your 
change on others, not just your own motivation.

If you do that you need to at least UNDERSTAND what i'm talking about. It scares the 
heck out of me that we are here at mail #20 with a *very* simple compatibility 
scenario and the maintainer of a major piece of free software infrastructure *still* 
does not understand this very simple argument. How the heck will you be able to 
handle more complex cases of compatibility in a responsible way?

> [...] Assembly programmers should appreciate this assembler change for helping 
> him/her catch such bugs in his/her codes.

Sure, and a warning will have a similar effect and we'll appreciate it. Forcing a 
build error and retroactively breaking the build for working code out of blue, not 
so much.

Thanks,

	Ingo

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[H.J. Lu]

On Wed, Mar 9, 2011 at 11:17 PM, Ingo Molnar <mingo@elte.hu> wrote:

> Your "it's simple to fix" argument misses that point: i was talking about bisection
> breakage, and during bisection of kernel bugs we dive back into older versions of
> the code.
>
> It does not matter that you declare the .size directive bogus. It is buggy and yes
> i've already queued up the fixes and i apprecitate them being fixed, but that's
> immaterial to the compatibility argument i made: BINUTILS BUILT THE KERNEL FINE
> BEFORE.
>
> If you do changes to a major component of the build environment of thousands of free
> software projects you *must* consider the real-life compatbility effects of your
> change on others, not just your own motivation.
>
> If you do that you need to at least UNDERSTAND what i'm talking about. It scares the
> heck out of me that we are here at mail #20 with a *very* simple compatibility
> scenario and the maintainer of a major piece of free software infrastructure *still*
> does not understand this very simple argument. How the heck will you be able to
> handle more complex cases of compatibility in a responsible way?
>

If I understand it correctly, the issue here is in some cases, for whatever
reason, the assembly input files can't be changed even if they are incorrect.
On the other hand, this is the incorrect assembly input as far as assembler
is concern.  You can write a simple assembly wrapper to fix the assembly input
before sending it to the real assembler.  Yes, it may be a hassle.  I guess
it is the price you may have to pay when you can't fix your code.

FWIW, I can provide the assembler wrapper if needed.

-- 
H.J.

Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S? Debian's binutils/as?)

[Ingo Molnar]

* H.J. Lu <hjl.tools@gmail.com> wrote:

> If I understand it correctly, the issue here is in some cases, for whatever 
> reason, the assembly input files can't be changed even if they are incorrect.

Correct - not having to change the source code is *very* important during testing 
and in particular during bisection testing.

This binutils change broke the kernel build (and hence bisection) for over 130,000 
existing commits (none of which can be changed - history is append-only immutable), 
on CONFIG_XEN=y x86-64 kernels.

Kernel bisection works like this:

   git bisect good v2.6.27
   git bisect  bad v2.6.37

The kernel gets rebuilt and tested by the tester at every commit that the bisection 
mechanism comes up with. Mid-section commits get marked either 'git bisect bad' or 
'git bisect good', depending on whether the bug is experienced or not.

Try it on a kernel repo, you can clone/track it:

   http://people.redhat.com/mingo/tip.git/README

The Linux kernel has over 200,000 commits currently, with about 10,000 new commits 
per new release. Bisection can easily go 'back in time' a few ten thousand commits 
and can end up on any of those commits (depending on the kind of bug that is being 
bisected).

You can try out the commands above. Please try it and mark commits good/bad randomly 
and see where you end up. You can end up *anywhere* within the 130,000+ commits 
bisection windo.

This CONFIG_XEN=y example shows why it's a *seriously* bad idea for build 
infrastructure to introduce build failures out of the blue. We want build
*warnings* to allow code that built fine before to still build.

Thanks,

	Ingo