This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Remove unnecessary and harmful fixincludes for Android

Hi Andrew, Joseph,
thanks for looking at the patch. See my comments and updated patch below.

2014-08-05 0:54 GMT+04:00 Andrew Pinski <pinskia@gmail.com>:
> On Mon, Aug 4, 2014 at 8:29 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
>> Hi,
>>
>> The following patch disables "stdio_va_list" fix: stdio.h is already
>> good in Android and, since ndk gcc is indented to be used with
>> different Android sysroots, it is actually harmful, because without
>> this fix only the version of stdio.h from the sysroot the compiler was
>> built with will be used.
>
> Isn't that why fixincludes is installed to run after the fact on sysroot?
>
> Thanks,
> Andrew Pinski
>

Are you saying that when you specify "--sysroot" option to an
installed compiler, it will "fixinclude" the headers from that sysroot
during the compilation process?
I don't see that happening. we see that on Android stdio.h is taken
only from the sysroot the compiler was built with.

2014-08-05 0:50 GMT+04:00 Joseph S. Myers <joseph@codesourcery.com>:
> On Mon, 4 Aug 2014, Alexander Ivchenko wrote:
>
>> +2014-08-04  Alexander Ivchenko  <alexander.ivchenko@intel.com>
>> +
>> + * inclhack.def (stdio_va_list): Disable fix for *android*.
>
> Testing for *android* is less than ideal, because of the possibility of
> configuring a *-linux* toolchain to have multilibs using various different
> C libraries (with -mandroid being used to select the Android multilib).
> So, specifying a bypass based on some relevant text that appears in the
> header would be better.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com

I've added check for "BIONIC" keyword. Hopefully it won't disappear.

Updated patch:

diff --git a/fixincludes/ChangeLog b/fixincludes/ChangeLog
index f7effee..e05412e 100644
--- a/fixincludes/ChangeLog
+++ b/fixincludes/ChangeLog
@@ -1,3 +1,10 @@
+2014-08-04  Alexander Ivchenko  <alexander.ivchenko@intel.com>
+
+ * inclhack.def (stdio_va_list): Bypass fix for Bionic.
+ (complier_h_tradcpp): Remove.
+ * fixincl.x: Regenerate.
+ * tests/base/linux/compiler.h: Remove.
+
 2014-04-22  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

  * inclhack.def (math_exception): Bypass on *-*-solaris2.1[0-9]*.
diff --git a/fixincludes/fixincl.x b/fixincludes/fixincl.x
index dd45802..d555c51 100644
--- a/fixincludes/fixincl.x
+++ b/fixincludes/fixincl.x
@@ -2,11 +2,11 @@
  *
  * DO NOT EDIT THIS FILE   (fixincl.x)
  *
- * It has been AutoGen-ed  Tuesday January  7, 2014 at 12:02:54 PM MET
+ * It has been AutoGen-ed  August  5, 2014 at 02:53:14 PM by AutoGen 5.12
  * From the definitions    inclhack.def
  * and the template file   fixincl
  */
-/* DO NOT SVN-MERGE THIS FILE, EITHER Tue Jan  7 12:02:54 MET 2014
+/* DO NOT SVN-MERGE THIS FILE, EITHER Tue Aug  5 14:53:14 MSK 2014
  *
  * You must regenerate it.  Use the ./genfixes script.
  *
@@ -15,7 +15,7 @@
  * certain ANSI-incompatible system header files which are fixed to work
  * correctly with ANSI C and placed in a directory that GNU C will search.
  *
- * This file contains 224 fixup descriptions.
+ * This file contains 223 fixup descriptions.
  *
  * See README for more information.
  *
@@ -2111,41 +2111,6 @@ int vfscanf(FILE *, const char *,
__builtin_va_list) __asm__ (_BSD_STRING(__USER

 /* * * * * * * * * * * * * * * * * * * * * * * * * *
  *
- *  Description of Complier_H_Tradcpp fix
- */
-tSCC zComplier_H_TradcppName[] =
-     "complier_h_tradcpp";
-
-/*
- *  File name selection pattern
- */
-tSCC zComplier_H_TradcppList[] =
-  "linux/compiler.h\0";
-/*
- *  Machine/OS name selection pattern
- */
-#define apzComplier_H_TradcppMachs (const char**)NULL
-
-/*
- *  content selection pattern - do fix if pattern found
- */
-tSCC zComplier_H_TradcppSelect0[] =
-       "#define __builtin_warning\\(x, y\\.\\.\\.\\) \\(1\\)";
-
-#define    COMPLIER_H_TRADCPP_TEST_CT  1
-static tTestDesc aComplier_H_TradcppTests[] = {
-  { TT_EGREP,    zComplier_H_TradcppSelect0, (regex_t*)NULL }, };
-
-/*
- *  Fix Command Arguments for Complier_H_Tradcpp
- */
-static const char* apzComplier_H_TradcppPatch[] = {
-    "format",
-    "/* __builtin_warning(x, y...) is obsolete */",
-    (char*)NULL };
-
-/* * * * * * * * * * * * * * * * * * * * * * * * * *
- *
  *  Description of Ctrl_Quotes_Def fix
  */
 tSCC zCtrl_Quotes_DefName[] =
@@ -7234,7 +7199,7 @@ tSCC* apzStdio_Va_ListMachs[] = {
  *  content bypass pattern - skip fix if pattern found
  */
 tSCC zStdio_Va_ListBypass0[] =
-       "__gnuc_va_list|_BSD_VA_LIST_|__DJ_va_list|_G_va_list";
+       "__gnuc_va_list|_BSD_VA_LIST_|__DJ_va_list|_G_va_list|BIONIC";

 #define    STDIO_VA_LIST_TEST_CT  1
 static tTestDesc aStdio_Va_ListTests[] = {
@@ -9187,9 +9152,9 @@ static const char* apzX11_SprintfPatch[] = {
  *
  *  List of all fixes
  */
-#define REGEX_COUNT          261
+#define REGEX_COUNT          260
 #define MACH_LIST_SIZE_LIMIT 187
-#define FIX_COUNT            224
+#define FIX_COUNT            223

 /*
  *  Enumerate the fixes
@@ -9242,7 +9207,6 @@ typedef enum {
     BROKEN_CABS_FIXIDX,
     BROKEN_NAN_FIXIDX,
     BSD_STDIO_ATTRS_CONFLICT_FIXIDX,
-    COMPLIER_H_TRADCPP_FIXIDX,
     CTRL_QUOTES_DEF_FIXIDX,
     CTRL_QUOTES_USE_FIXIDX,
     CXX_UNREADY_FIXIDX,
@@ -9657,11 +9621,6 @@ tFixDesc fixDescList[ FIX_COUNT ] = {
      BSD_STDIO_ATTRS_CONFLICT_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
      aBsd_Stdio_Attrs_ConflictTests,   apzBsd_Stdio_Attrs_ConflictPatch, 0 },

-  {  zComplier_H_TradcppName,    zComplier_H_TradcppList,
-     apzComplier_H_TradcppMachs,
-     COMPLIER_H_TRADCPP_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
-     aComplier_H_TradcppTests,   apzComplier_H_TradcppPatch, 0 },
-
   {  zCtrl_Quotes_DefName,    zCtrl_Quotes_DefList,
      apzCtrl_Quotes_DefMachs,
      CTRL_QUOTES_DEF_TEST_CT, FD_MACH_ONLY | FD_SUBROUTINE,
diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
index 6a1136c..bf452c6 100644
--- a/fixincludes/inclhack.def
+++ b/fixincludes/inclhack.def
@@ -1140,20 +1140,6 @@ fix = {
 };

 /*
- *  Old Linux kernel's <compiler.h> header breaks Traditional CPP
- */
-fix = {
-    hackname  = complier_h_tradcpp;
-    files     = linux/compiler.h;
-
-    select    = "#define __builtin_warning\\(x, y\\.\\.\\.\\) \\(1\\)";
-    c_fix     = format;
-    c_fix_arg = "/* __builtin_warning(x, y...) is obsolete */";
-
-    test_text = "#define __builtin_warning(x, y...) (1)";
-};
-
-/*
  *  Fix various macros used to define ioctl numbers.
  *  The traditional syntax was:
  *
@@ -3722,8 +3708,9 @@ fix = {
 fix = {
     hackname = stdio_va_list;
     files    = stdio.h;
-    bypass   = '__gnuc_va_list|_BSD_VA_LIST_|__DJ_va_list|_G_va_list';
+    bypass   = '__gnuc_va_list|_BSD_VA_LIST_|__DJ_va_list|_G_va_list|BIONIC';
     /*
+     * In Bionic va_list is always appropriately typedefed to __gnuc_va_list.
      * On Solaris 10, the definition in
      * <stdio.h> is guarded appropriately by the _XPG4 feature macro;
      * there is therefore no need for this fix there.
diff --git a/fixincludes/tests/base/linux/compiler.h
b/fixincludes/tests/base/linux/compiler.h
deleted file mode 100644
index 7135276..0000000
--- a/fixincludes/tests/base/linux/compiler.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/*  DO NOT EDIT THIS FILE.
-
-    It has been auto-edited by fixincludes from:
-
- "fixinc/tests/inc/linux/compiler.h"
-
-    This had to be done to correct non-standard usages in the
-    original, manufacturer supplied header file.  */
-
-
-
-#if defined( COMPLIER_H_TRADCPP_CHECK )
-/* __builtin_warning(x, y...) is obsolete */
-#endif  /* COMPLIER_H_TRADCPP_CHECK */



Is it ok?

--Alexander
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]