* [PATCH v3 1/5] sim: Remove self-assignments
2022-10-06 6:43 ` [PATCH v3 0/5] sim: Suppress warnings if built with Clang Tsukasa OI
@ 2022-10-06 6:43 ` Tsukasa OI
2022-10-11 14:21 ` Andrew Burgess
2022-10-06 6:43 ` [PATCH v3 2/5] sim: Make WITH_{TRACE,PROFILE}-based macros bool Tsukasa OI
` (5 subsequent siblings)
6 siblings, 1 reply; 38+ messages in thread
From: Tsukasa OI @ 2022-10-06 6:43 UTC (permalink / raw)
To: Tsukasa OI, Andrew Burgess, Mike Frysinger; +Cc: gdb-patches
Clang generates a warning if there is a redundant self-assignment
("-Wself-assign"). On the default configuration, it causes a build failure
(unless "--disable-werror" is specified).
This commit removes two redundant self-assignments.
---
sim/common/hw-tree.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/sim/common/hw-tree.c b/sim/common/hw-tree.c
index 56319333d76..8bb5ac77545 100644
--- a/sim/common/hw-tree.c
+++ b/sim/common/hw-tree.c
@@ -335,7 +335,6 @@ split_find_device (struct hw *current,
else if (strncmp (spec->path, "./", strlen ("./")) == 0)
{
/* cd ./... */
- current = current;
spec->path += strlen ("./");
}
else if (strncmp (spec->path, "../", strlen ("../")) == 0)
@@ -348,7 +347,6 @@ split_find_device (struct hw *current,
else if (strcmp (spec->path, ".") == 0)
{
/* cd . */
- current = current;
spec->path += strlen (".");
}
else if (strcmp (spec->path, "..") == 0)
--
2.34.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 1/5] sim: Remove self-assignments
2022-10-06 6:43 ` [PATCH v3 1/5] sim: Remove self-assignments Tsukasa OI
@ 2022-10-11 14:21 ` Andrew Burgess
2022-10-11 14:29 ` Tsukasa OI
0 siblings, 1 reply; 38+ messages in thread
From: Andrew Burgess @ 2022-10-11 14:21 UTC (permalink / raw)
To: Tsukasa OI, Tsukasa OI, Mike Frysinger; +Cc: gdb-patches
Tsukasa OI <research_trasio@irq.a4lg.com> writes:
> Clang generates a warning if there is a redundant self-assignment
> ("-Wself-assign"). On the default configuration, it causes a build failure
> (unless "--disable-werror" is specified).
>
> This commit removes two redundant self-assignments.
I pushed this patch, but I found a couple more of these in ppc/tree.c
which I included in this commit.
Thanks,
Andrew
---
commit 5294d882ebb5b10f8a246f2c51ffef05622ef16c
Author: Tsukasa OI <research_trasio@irq.a4lg.com>
Date: Sun Sep 25 08:42:02 2022 +0000
sim: Remove self-assignments
Clang generates a warning if there is a redundant self-assignment
("-Wself-assign"). On the default configuration, it causes a build failure
(unless "--disable-werror" is specified).
This commit removes redundant self-assignments from two files.
diff --git a/sim/common/hw-tree.c b/sim/common/hw-tree.c
index 56319333d76..8bb5ac77545 100644
--- a/sim/common/hw-tree.c
+++ b/sim/common/hw-tree.c
@@ -335,7 +335,6 @@ split_find_device (struct hw *current,
else if (strncmp (spec->path, "./", strlen ("./")) == 0)
{
/* cd ./... */
- current = current;
spec->path += strlen ("./");
}
else if (strncmp (spec->path, "../", strlen ("../")) == 0)
@@ -348,7 +347,6 @@ split_find_device (struct hw *current,
else if (strcmp (spec->path, ".") == 0)
{
/* cd . */
- current = current;
spec->path += strlen (".");
}
else if (strcmp (spec->path, "..") == 0)
diff --git a/sim/ppc/tree.c b/sim/ppc/tree.c
index 6d20665505e..05532bb47ee 100644
--- a/sim/ppc/tree.c
+++ b/sim/ppc/tree.c
@@ -306,7 +306,6 @@ split_find_device(device *current,
}
else if (strncmp(spec->path, "./", strlen("./")) == 0) {
/* cd ./... */
- current = current;
spec->path += strlen("./");
}
else if (strncmp(spec->path, "../", strlen("../")) == 0) {
@@ -317,7 +316,6 @@ split_find_device(device *current,
}
else if (strcmp(spec->path, ".") == 0) {
/* cd . */
- current = current;
spec->path += strlen(".");
}
else if (strcmp(spec->path, "..") == 0) {
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 1/5] sim: Remove self-assignments
2022-10-11 14:21 ` Andrew Burgess
@ 2022-10-11 14:29 ` Tsukasa OI
0 siblings, 0 replies; 38+ messages in thread
From: Tsukasa OI @ 2022-10-11 14:29 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 2022/10/11 23:21, Andrew Burgess wrote:
> Tsukasa OI <research_trasio@irq.a4lg.com> writes:
>
>> Clang generates a warning if there is a redundant self-assignment
>> ("-Wself-assign"). On the default configuration, it causes a build failure
>> (unless "--disable-werror" is specified).
>>
>> This commit removes two redundant self-assignments.
>
> I pushed this patch, but I found a couple more of these in ppc/tree.c
> which I included in this commit.
>
> Thanks,
> Andrew
My another (upcoming) patchset included exact changes to sim/ppc/tree.c
but thanks for adding it anyway.
Thanks,
Tsukasa
>
> ---
>
> commit 5294d882ebb5b10f8a246f2c51ffef05622ef16c
> Author: Tsukasa OI <research_trasio@irq.a4lg.com>
> Date: Sun Sep 25 08:42:02 2022 +0000
>
> sim: Remove self-assignments
>
> Clang generates a warning if there is a redundant self-assignment
> ("-Wself-assign"). On the default configuration, it causes a build failure
> (unless "--disable-werror" is specified).
>
> This commit removes redundant self-assignments from two files.
>
> diff --git a/sim/common/hw-tree.c b/sim/common/hw-tree.c
> index 56319333d76..8bb5ac77545 100644
> --- a/sim/common/hw-tree.c
> +++ b/sim/common/hw-tree.c
> @@ -335,7 +335,6 @@ split_find_device (struct hw *current,
> else if (strncmp (spec->path, "./", strlen ("./")) == 0)
> {
> /* cd ./... */
> - current = current;
> spec->path += strlen ("./");
> }
> else if (strncmp (spec->path, "../", strlen ("../")) == 0)
> @@ -348,7 +347,6 @@ split_find_device (struct hw *current,
> else if (strcmp (spec->path, ".") == 0)
> {
> /* cd . */
> - current = current;
> spec->path += strlen (".");
> }
> else if (strcmp (spec->path, "..") == 0)
> diff --git a/sim/ppc/tree.c b/sim/ppc/tree.c
> index 6d20665505e..05532bb47ee 100644
> --- a/sim/ppc/tree.c
> +++ b/sim/ppc/tree.c
> @@ -306,7 +306,6 @@ split_find_device(device *current,
> }
> else if (strncmp(spec->path, "./", strlen("./")) == 0) {
> /* cd ./... */
> - current = current;
> spec->path += strlen("./");
> }
> else if (strncmp(spec->path, "../", strlen("../")) == 0) {
> @@ -317,7 +316,6 @@ split_find_device(device *current,
> }
> else if (strcmp(spec->path, ".") == 0) {
> /* cd . */
> - current = current;
> spec->path += strlen(".");
> }
> else if (strcmp(spec->path, "..") == 0) {
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 2/5] sim: Make WITH_{TRACE,PROFILE}-based macros bool
2022-10-06 6:43 ` [PATCH v3 0/5] sim: Suppress warnings if built with Clang Tsukasa OI
2022-10-06 6:43 ` [PATCH v3 1/5] sim: Remove self-assignments Tsukasa OI
@ 2022-10-06 6:43 ` Tsukasa OI
2022-10-06 6:43 ` [PATCH v3 3/5] sim: Suppress non-literal printf warning Tsukasa OI
` (4 subsequent siblings)
6 siblings, 0 replies; 38+ messages in thread
From: Tsukasa OI @ 2022-10-06 6:43 UTC (permalink / raw)
To: Tsukasa OI, Andrew Burgess, Mike Frysinger; +Cc: gdb-patches
Clang generates a warning if there is an ambiguous expression (possibly a
bitwise operation (& or |), but a logical operator (&& or ||) is used;
"-Wconstant-logical-operand"). On the default configuration, it causes a
build failure (unless "--disable-werror" is specified).
This is caused by predicate macros that use the form (base_variable & flag).
Clang considers them as regular integer values (not boolean) and
generates that warning.
This commit makes Clang think those predicate macros to be boolean.
---
sim/common/sim-profile.h | 12 ++++++------
sim/common/sim-trace.h | 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/sim/common/sim-profile.h b/sim/common/sim-profile.h
index 61d5039e10d..d072621a514 100644
--- a/sim/common/sim-profile.h
+++ b/sim/common/sim-profile.h
@@ -87,12 +87,12 @@ SIM_RC sim_profile_set_option (SIM_DESC sd_, const char *name_, int idx_,
#define PROFILE_core (1 << PROFILE_CORE_IDX)
/* Preprocessor macros to simplify tests of WITH_PROFILE. */
-#define WITH_PROFILE_INSN_P (WITH_PROFILE & PROFILE_insn)
-#define WITH_PROFILE_MEMORY_P (WITH_PROFILE & PROFILE_memory)
-#define WITH_PROFILE_MODEL_P (WITH_PROFILE & PROFILE_model)
-#define WITH_PROFILE_SCACHE_P (WITH_PROFILE & PROFILE_scache)
-#define WITH_PROFILE_PC_P (WITH_PROFILE & PROFILE_pc)
-#define WITH_PROFILE_CORE_P (WITH_PROFILE & PROFILE_core)
+#define WITH_PROFILE_INSN_P ((WITH_PROFILE & PROFILE_insn) != 0)
+#define WITH_PROFILE_MEMORY_P ((WITH_PROFILE & PROFILE_memory) != 0)
+#define WITH_PROFILE_MODEL_P ((WITH_PROFILE & PROFILE_model) != 0)
+#define WITH_PROFILE_SCACHE_P ((WITH_PROFILE & PROFILE_scache) != 0)
+#define WITH_PROFILE_PC_P ((WITH_PROFILE & PROFILE_pc) != 0)
+#define WITH_PROFILE_CORE_P ((WITH_PROFILE & PROFILE_core) != 0)
/* If MAX_TARGET_MODES isn't defined, we can't do memory profiling.
??? It is intended that this is a temporary occurrence. Normally
diff --git a/sim/common/sim-trace.h b/sim/common/sim-trace.h
index d08810d9fcc..8d4ab130815 100644
--- a/sim/common/sim-trace.h
+++ b/sim/common/sim-trace.h
@@ -125,10 +125,10 @@ enum {
#define TRACE_debug (1 << TRACE_DEBUG_IDX)
/* Return non-zero if tracing of idx is enabled (compiled in). */
-#define WITH_TRACE_P(idx) (WITH_TRACE & (1 << idx))
+#define WITH_TRACE_P(idx) ((WITH_TRACE & (1 << idx)) != 0)
/* Preprocessor macros to simplify tests of WITH_TRACE. */
-#define WITH_TRACE_ANY_P (WITH_TRACE)
+#define WITH_TRACE_ANY_P (WITH_TRACE != 0)
#define WITH_TRACE_INSN_P WITH_TRACE_P (TRACE_INSN_IDX)
#define WITH_TRACE_DISASM_P WITH_TRACE_P (TRACE_DISASM_IDX)
#define WITH_TRACE_DECODE_P WITH_TRACE_P (TRACE_DECODE_IDX)
--
2.34.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 3/5] sim: Suppress non-literal printf warning
2022-10-06 6:43 ` [PATCH v3 0/5] sim: Suppress warnings if built with Clang Tsukasa OI
2022-10-06 6:43 ` [PATCH v3 1/5] sim: Remove self-assignments Tsukasa OI
2022-10-06 6:43 ` [PATCH v3 2/5] sim: Make WITH_{TRACE,PROFILE}-based macros bool Tsukasa OI
@ 2022-10-06 6:43 ` Tsukasa OI
2022-10-11 14:22 ` Andrew Burgess
2022-10-06 6:43 ` [PATCH v3 4/5] sim: Check known getopt definition existence Tsukasa OI
` (3 subsequent siblings)
6 siblings, 1 reply; 38+ messages in thread
From: Tsukasa OI @ 2022-10-06 6:43 UTC (permalink / raw)
To: Tsukasa OI, Andrew Burgess, Mike Frysinger; +Cc: gdb-patches
Clang generates a warning if the format string of a printf-like function is
not a literal ("-Wformat-nonliteral"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
To avoid this warning, this commit now uses vsnprintf to format error
message and pass the message to sim_engine_abort function with another
printf-style formatting.
This patch is mostly authored by Andrew Burgess and slightly modified by
Tsukasa OI.
Co-authored-by: Andrew Burgess <aburgess@redhat.com>
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
sim/common/sim-hw.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/sim/common/sim-hw.c b/sim/common/sim-hw.c
index cece5638bc9..c70770893c9 100644
--- a/sim/common/sim-hw.c
+++ b/sim/common/sim-hw.c
@@ -403,13 +403,16 @@ sim_hw_io_write_buffer (struct sim_state *sd,
\f
/* Abort the simulation specifying HW as the reason */
-void
+void ATTRIBUTE_PRINTF (2, 0)
hw_vabort (struct hw *me,
const char *fmt,
va_list ap)
{
+ int len;
const char *name;
char *msg;
+ va_list cpy;
+
/* find an identity */
if (me != NULL && hw_path (me) != NULL && hw_path (me) [0] != '\0')
name = hw_path (me);
@@ -419,16 +422,19 @@ hw_vabort (struct hw *me,
name = hw_family (me);
else
name = "device";
- /* construct an updated format string */
- msg = alloca (strlen (name) + strlen (": ") + strlen (fmt) + 1);
- strcpy (msg, name);
- strcat (msg, ": ");
- strcat (msg, fmt);
+
+ /* Expand FMT and AP into MSG buffer. */
+ va_copy (cpy, ap);
+ len = vsnprintf (NULL, 0, fmt, cpy) + 1;
+ va_end (cpy);
+ msg = alloca (len);
+ vsnprintf (msg, len, fmt, ap);
+
/* report the problem */
- sim_engine_vabort (hw_system (me),
- STATE_HW (hw_system (me))->cpu,
- STATE_HW (hw_system (me))->cia,
- msg, ap);
+ sim_engine_abort (hw_system (me),
+ STATE_HW (hw_system (me))->cpu,
+ STATE_HW (hw_system (me))->cia,
+ "%s: %s", name, msg);
}
void
--
2.34.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 3/5] sim: Suppress non-literal printf warning
2022-10-06 6:43 ` [PATCH v3 3/5] sim: Suppress non-literal printf warning Tsukasa OI
@ 2022-10-11 14:22 ` Andrew Burgess
0 siblings, 0 replies; 38+ messages in thread
From: Andrew Burgess @ 2022-10-11 14:22 UTC (permalink / raw)
To: Tsukasa OI, Tsukasa OI, Mike Frysinger; +Cc: gdb-patches
Tsukasa OI <research_trasio@irq.a4lg.com> writes:
> Clang generates a warning if the format string of a printf-like function is
> not a literal ("-Wformat-nonliteral"). On the default configuration, it
> causes a build failure (unless "--disable-werror" is specified).
>
> To avoid this warning, this commit now uses vsnprintf to format error
> message and pass the message to sim_engine_abort function with another
> printf-style formatting.
>
> This patch is mostly authored by Andrew Burgess and slightly modified by
> Tsukasa OI.
>
> Co-authored-by: Andrew Burgess <aburgess@redhat.com>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> ---
> sim/common/sim-hw.c | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/sim/common/sim-hw.c b/sim/common/sim-hw.c
> index cece5638bc9..c70770893c9 100644
> --- a/sim/common/sim-hw.c
> +++ b/sim/common/sim-hw.c
> @@ -403,13 +403,16 @@ sim_hw_io_write_buffer (struct sim_state *sd,
> \f
> /* Abort the simulation specifying HW as the reason */
>
> -void
> +void ATTRIBUTE_PRINTF (2, 0)
> hw_vabort (struct hw *me,
> const char *fmt,
> va_list ap)
I've pushed this patch, but I moved the ATTRIBUTE_PRINTF to the function
declaration.
Thanks,
Andrew
---
commit 96894c19ad2b91db76b9b606d48c56ad354b4801
Author: Tsukasa OI <research_trasio@irq.a4lg.com>
Date: Thu Oct 6 06:43:51 2022 +0000
sim: Suppress non-literal printf warning
Clang generates a warning if the format string of a printf-like function is
not a literal ("-Wformat-nonliteral"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
To avoid this warning, this commit now uses vsnprintf to format error
message and pass the message to sim_engine_abort function with another
printf-style formatting.
This patch is mostly authored by Andrew Burgess and slightly modified by
Tsukasa OI.
Co-authored-by: Andrew Burgess <aburgess@redhat.com>
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
diff --git a/sim/common/hw-device.h b/sim/common/hw-device.h
index 7a36f2f518c..451f88723de 100644
--- a/sim/common/hw-device.h
+++ b/sim/common/hw-device.h
@@ -437,10 +437,8 @@ void hw_abort
const char *fmt,
...) ATTRIBUTE_PRINTF (2, 3) ATTRIBUTE_NORETURN;
-void hw_vabort
-(struct hw *me,
- const char *fmt,
- va_list ap) ATTRIBUTE_NORETURN;
+extern void hw_vabort (struct hw *me, const char *fmt, va_list ap)
+ ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (2, 0);
void hw_halt
(struct hw *me,
diff --git a/sim/common/sim-hw.c b/sim/common/sim-hw.c
index cece5638bc9..7bfe91e4ae2 100644
--- a/sim/common/sim-hw.c
+++ b/sim/common/sim-hw.c
@@ -408,8 +408,11 @@ hw_vabort (struct hw *me,
const char *fmt,
va_list ap)
{
+ int len;
const char *name;
char *msg;
+ va_list cpy;
+
/* find an identity */
if (me != NULL && hw_path (me) != NULL && hw_path (me) [0] != '\0')
name = hw_path (me);
@@ -419,16 +422,19 @@ hw_vabort (struct hw *me,
name = hw_family (me);
else
name = "device";
- /* construct an updated format string */
- msg = alloca (strlen (name) + strlen (": ") + strlen (fmt) + 1);
- strcpy (msg, name);
- strcat (msg, ": ");
- strcat (msg, fmt);
+
+ /* Expand FMT and AP into MSG buffer. */
+ va_copy (cpy, ap);
+ len = vsnprintf (NULL, 0, fmt, cpy) + 1;
+ va_end (cpy);
+ msg = alloca (len);
+ vsnprintf (msg, len, fmt, ap);
+
/* report the problem */
- sim_engine_vabort (hw_system (me),
- STATE_HW (hw_system (me))->cpu,
- STATE_HW (hw_system (me))->cia,
- msg, ap);
+ sim_engine_abort (hw_system (me),
+ STATE_HW (hw_system (me))->cpu,
+ STATE_HW (hw_system (me))->cia,
+ "%s: %s", name, msg);
}
void
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 4/5] sim: Check known getopt definition existence
2022-10-06 6:43 ` [PATCH v3 0/5] sim: Suppress warnings if built with Clang Tsukasa OI
` (2 preceding siblings ...)
2022-10-06 6:43 ` [PATCH v3 3/5] sim: Suppress non-literal printf warning Tsukasa OI
@ 2022-10-06 6:43 ` Tsukasa OI
2022-10-12 16:28 ` Tom de Vries
2022-10-23 12:16 ` Mike Frysinger
2022-10-06 6:43 ` [PATCH v3 5/5] sim: Initialize pbb_br_* by default Tsukasa OI
` (2 subsequent siblings)
6 siblings, 2 replies; 38+ messages in thread
From: Tsukasa OI @ 2022-10-06 6:43 UTC (permalink / raw)
To: Tsukasa OI, Andrew Burgess, Mike Frysinger; +Cc: gdb-patches
Clang generates a warning if there is a function declaration/definition
with zero arguments. Such declarations/definitions without a prototype (an
argument list) are deprecated forms of indefinite arguments
("-Wdeprecated-non-prototype"). On the default configuration, it causes a
build failure (unless "--disable-werror" is specified).
include/getopt.h defines some getopt function definitions but one of them
has a form "extern int getopt ();". If this form is selected in
include/getopt.h, Clang generates a warning and the build fails by default.
In really old environments, this getopt definition with no arguments is
necessary (because the definition may change between environments).
However, this definition is now a cause of problems on modern environments.
A good news is, this definition is not always selected (e.g. if used by
binutils/*.c). This is because configuration scripts of binutils, gas,
gprof and ld tries to find known definition of getopt function is used and
defines HAVE_DECL_GETOPT macro. If this macro is defined when getopt.h is
included, a good form of getopt is used and Clang won't generate warnings.
This commit adds a modified portion of ld/configure.ac to find the known
getopt definition. If we could find one (and we *will* in most modern
environments), we don't need to rely on the deprecated definition.
---
sim/config.h.in | 3 +++
sim/configure | 32 ++++++++++++++++++++++++++++++++
sim/configure.ac | 10 ++++++++++
3 files changed, 45 insertions(+)
diff --git a/sim/config.h.in b/sim/config.h.in
index 84c363c0aec..9a94b289e46 100644
--- a/sim/config.h.in
+++ b/sim/config.h.in
@@ -41,6 +41,9 @@
/* Define to 1 if you have the `chmod' function. */
#undef HAVE_CHMOD
+/* Is the prototype for getopt in <unistd.h> in the expected format? */
+#undef HAVE_DECL_GETOPT
+
/* Define to 1 if you have the declaration of `tzname', and to 0 if you don't.
*/
#undef HAVE_DECL_TZNAME
diff --git a/sim/configure b/sim/configure
index 75d1935df38..dac7f085be1 100755
--- a/sim/configure
+++ b/sim/configure
@@ -16428,6 +16428,38 @@ $as_echo "${WARN_CFLAGS} ${WERROR_CFLAGS}" >&6; }
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for a known getopt prototype in unistd.h" >&5
+$as_echo_n "checking for a known getopt prototype in unistd.h... " >&6; }
+if ${sim_cv_decl_getopt_unistd_h+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+#include <unistd.h>
+int
+main ()
+{
+extern int getopt (int, char *const*, const char *);
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ sim_cv_decl_getopt_unistd_h=yes
+else
+ sim_cv_decl_getopt_unistd_h=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $sim_cv_decl_getopt_unistd_h" >&5
+$as_echo "$sim_cv_decl_getopt_unistd_h" >&6; }
+if test $sim_cv_decl_getopt_unistd_h = yes; then
+
+$as_echo "#define HAVE_DECL_GETOPT 1" >>confdefs.h
+
+fi
+
diff --git a/sim/configure.ac b/sim/configure.ac
index 66a1020efe0..be0cfdbea32 100644
--- a/sim/configure.ac
+++ b/sim/configure.ac
@@ -177,6 +177,16 @@ SIM_AC_OPTION_STDIO
SIM_AC_OPTION_TRACE
SIM_AC_OPTION_WARNINGS
+AC_MSG_CHECKING(for a known getopt prototype in unistd.h)
+AC_CACHE_VAL(sim_cv_decl_getopt_unistd_h,
+[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([#include <unistd.h>], [extern int getopt (int, char *const*, const char *);])],
+sim_cv_decl_getopt_unistd_h=yes, sim_cv_decl_getopt_unistd_h=no)])
+AC_MSG_RESULT($sim_cv_decl_getopt_unistd_h)
+if test $sim_cv_decl_getopt_unistd_h = yes; then
+ AC_DEFINE([HAVE_DECL_GETOPT], 1,
+ [Is the prototype for getopt in <unistd.h> in the expected format?])
+fi
+
dnl These are unfortunate. They are conditionally called by other sim macros
dnl but always used by common/Make-common.in. So we have to subst here even
dnl when the rest of the code is in the respective macros. Once we merge the
--
2.34.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/5] sim: Check known getopt definition existence
2022-10-06 6:43 ` [PATCH v3 4/5] sim: Check known getopt definition existence Tsukasa OI
@ 2022-10-12 16:28 ` Tom de Vries
2022-10-12 17:03 ` Tsukasa OI
2022-10-13 9:50 ` Tsukasa OI
2022-10-23 12:16 ` Mike Frysinger
1 sibling, 2 replies; 38+ messages in thread
From: Tom de Vries @ 2022-10-12 16:28 UTC (permalink / raw)
To: Tsukasa OI, Andrew Burgess, Mike Frysinger; +Cc: gdb-patches
On 10/6/22 08:43, Tsukasa OI via Gdb-patches wrote:
> Clang generates a warning if there is a function declaration/definition
> with zero arguments. Such declarations/definitions without a prototype (an
> argument list) are deprecated forms of indefinite arguments
> ("-Wdeprecated-non-prototype"). On the default configuration, it causes a
> build failure (unless "--disable-werror" is specified).
>
> include/getopt.h defines some getopt function definitions but one of them
> has a form "extern int getopt ();". If this form is selected in
> include/getopt.h, Clang generates a warning and the build fails by default.
>
> In really old environments, this getopt definition with no arguments is
> necessary (because the definition may change between environments).
> However, this definition is now a cause of problems on modern environments.
>
> A good news is, this definition is not always selected (e.g. if used by
> binutils/*.c). This is because configuration scripts of binutils, gas,
> gprof and ld tries to find known definition of getopt function is used and
> defines HAVE_DECL_GETOPT macro. If this macro is defined when getopt.h is
> included, a good form of getopt is used and Clang won't generate warnings.
>
> This commit adds a modified portion of ld/configure.ac to find the known
> getopt definition. If we could find one (and we *will* in most modern
> environments), we don't need to rely on the deprecated definition.
I'm guessing this cause the build breakage on buildbot gdb-centos-x86_64 .
https://builder.sourceware.org/buildbot/#/builders/71/builds/1392
Thanks,
- Tom
> ---
> sim/config.h.in | 3 +++
> sim/configure | 32 ++++++++++++++++++++++++++++++++
> sim/configure.ac | 10 ++++++++++
> 3 files changed, 45 insertions(+)
>
> diff --git a/sim/config.h.in b/sim/config.h.in
> index 84c363c0aec..9a94b289e46 100644
> --- a/sim/config.h.in
> +++ b/sim/config.h.in
> @@ -41,6 +41,9 @@
> /* Define to 1 if you have the `chmod' function. */
> #undef HAVE_CHMOD
>
> +/* Is the prototype for getopt in <unistd.h> in the expected format? */
> +#undef HAVE_DECL_GETOPT
> +
> /* Define to 1 if you have the declaration of `tzname', and to 0 if you don't.
> */
> #undef HAVE_DECL_TZNAME
> diff --git a/sim/configure b/sim/configure
> index 75d1935df38..dac7f085be1 100755
> --- a/sim/configure
> +++ b/sim/configure
> @@ -16428,6 +16428,38 @@ $as_echo "${WARN_CFLAGS} ${WERROR_CFLAGS}" >&6; }
> fi
>
>
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for a known getopt prototype in unistd.h" >&5
> +$as_echo_n "checking for a known getopt prototype in unistd.h... " >&6; }
> +if ${sim_cv_decl_getopt_unistd_h+:} false; then :
> + $as_echo_n "(cached) " >&6
> +else
> + cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h. */
> +#include <unistd.h>
> +int
> +main ()
> +{
> +extern int getopt (int, char *const*, const char *);
> + ;
> + return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_compile "$LINENO"; then :
> + sim_cv_decl_getopt_unistd_h=yes
> +else
> + sim_cv_decl_getopt_unistd_h=no
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> +fi
> +
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $sim_cv_decl_getopt_unistd_h" >&5
> +$as_echo "$sim_cv_decl_getopt_unistd_h" >&6; }
> +if test $sim_cv_decl_getopt_unistd_h = yes; then
> +
> +$as_echo "#define HAVE_DECL_GETOPT 1" >>confdefs.h
> +
> +fi
> +
>
>
>
> diff --git a/sim/configure.ac b/sim/configure.ac
> index 66a1020efe0..be0cfdbea32 100644
> --- a/sim/configure.ac
> +++ b/sim/configure.ac
> @@ -177,6 +177,16 @@ SIM_AC_OPTION_STDIO
> SIM_AC_OPTION_TRACE
> SIM_AC_OPTION_WARNINGS
>
> +AC_MSG_CHECKING(for a known getopt prototype in unistd.h)
> +AC_CACHE_VAL(sim_cv_decl_getopt_unistd_h,
> +[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([#include <unistd.h>], [extern int getopt (int, char *const*, const char *);])],
> +sim_cv_decl_getopt_unistd_h=yes, sim_cv_decl_getopt_unistd_h=no)])
> +AC_MSG_RESULT($sim_cv_decl_getopt_unistd_h)
> +if test $sim_cv_decl_getopt_unistd_h = yes; then
> + AC_DEFINE([HAVE_DECL_GETOPT], 1,
> + [Is the prototype for getopt in <unistd.h> in the expected format?])
> +fi
> +
> dnl These are unfortunate. They are conditionally called by other sim macros
> dnl but always used by common/Make-common.in. So we have to subst here even
> dnl when the rest of the code is in the respective macros. Once we merge the
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/5] sim: Check known getopt definition existence
2022-10-12 16:28 ` Tom de Vries
@ 2022-10-12 17:03 ` Tsukasa OI
2022-10-12 17:08 ` Tom de Vries
2022-10-13 9:50 ` Tsukasa OI
1 sibling, 1 reply; 38+ messages in thread
From: Tsukasa OI @ 2022-10-12 17:03 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches
Hi Tom,
On 2022/10/13 1:28, Tom de Vries wrote:
> On 10/6/22 08:43, Tsukasa OI via Gdb-patches wrote:
>> Clang generates a warning if there is a function declaration/definition
>> with zero arguments. Such declarations/definitions without a
>> prototype (an
>> argument list) are deprecated forms of indefinite arguments
>> ("-Wdeprecated-non-prototype"). On the default configuration, it
>> causes a
>> build failure (unless "--disable-werror" is specified).
>>
>> include/getopt.h defines some getopt function definitions but one of them
>> has a form "extern int getopt ();". If this form is selected in
>> include/getopt.h, Clang generates a warning and the build fails by
>> default.
>>
>> In really old environments, this getopt definition with no arguments is
>> necessary (because the definition may change between environments).
>> However, this definition is now a cause of problems on modern
>> environments.
>>
>> A good news is, this definition is not always selected (e.g. if used by
>> binutils/*.c). This is because configuration scripts of binutils, gas,
>> gprof and ld tries to find known definition of getopt function is used
>> and
>> defines HAVE_DECL_GETOPT macro. If this macro is defined when
>> getopt.h is
>> included, a good form of getopt is used and Clang won't generate
>> warnings.
>>
>> This commit adds a modified portion of ld/configure.ac to find the known
>> getopt definition. If we could find one (and we *will* in most modern
>> environments), we don't need to rely on the deprecated definition.
>
> I'm guessing this cause the build breakage on buildbot gdb-centos-x86_64 .
>
> https://builder.sourceware.org/buildbot/#/builders/71/builds/1392
>
> Thanks,
> - Tom
I didn't see that error and it seems this error is very strange
considering sim/m32c/main.c is the **only** file which should not be
affected by my change as long as <unistd.h> correctly declares getopt.
Adding
#include <getopt.h>
after
#include <sys/types.h>
might change something but I cannot guarantee whether this change would
fix the issue.
Thanks,
Tsukasa
>
>
>> ---
>> sim/config.h.in | 3 +++
>> sim/configure | 32 ++++++++++++++++++++++++++++++++
>> sim/configure.ac | 10 ++++++++++
>> 3 files changed, 45 insertions(+)
>>
>> diff --git a/sim/config.h.in b/sim/config.h.in
>> index 84c363c0aec..9a94b289e46 100644
>> --- a/sim/config.h.in
>> +++ b/sim/config.h.in
>> @@ -41,6 +41,9 @@
>> /* Define to 1 if you have the `chmod' function. */
>> #undef HAVE_CHMOD
>> +/* Is the prototype for getopt in <unistd.h> in the expected
>> format? */
>> +#undef HAVE_DECL_GETOPT
>> +
>> /* Define to 1 if you have the declaration of `tzname', and to 0 if
>> you don't.
>> */
>> #undef HAVE_DECL_TZNAME
>> diff --git a/sim/configure b/sim/configure
>> index 75d1935df38..dac7f085be1 100755
>> --- a/sim/configure
>> +++ b/sim/configure
>> @@ -16428,6 +16428,38 @@ $as_echo "${WARN_CFLAGS} ${WERROR_CFLAGS}"
>> >&6; }
>> fi
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for a known
>> getopt prototype in unistd.h" >&5
>> +$as_echo_n "checking for a known getopt prototype in unistd.h... "
>> >&6; }
>> +if ${sim_cv_decl_getopt_unistd_h+:} false; then :
>> + $as_echo_n "(cached) " >&6
>> +else
>> + cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>> +/* end confdefs.h. */
>> +#include <unistd.h>
>> +int
>> +main ()
>> +{
>> +extern int getopt (int, char *const*, const char *);
>> + ;
>> + return 0;
>> +}
>> +_ACEOF
>> +if ac_fn_c_try_compile "$LINENO"; then :
>> + sim_cv_decl_getopt_unistd_h=yes
>> +else
>> + sim_cv_decl_getopt_unistd_h=no
>> +fi
>> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>> +fi
>> +
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
>> $sim_cv_decl_getopt_unistd_h" >&5
>> +$as_echo "$sim_cv_decl_getopt_unistd_h" >&6; }
>> +if test $sim_cv_decl_getopt_unistd_h = yes; then
>> +
>> +$as_echo "#define HAVE_DECL_GETOPT 1" >>confdefs.h
>> +
>> +fi
>> +
>> diff --git a/sim/configure.ac b/sim/configure.ac
>> index 66a1020efe0..be0cfdbea32 100644
>> --- a/sim/configure.ac
>> +++ b/sim/configure.ac
>> @@ -177,6 +177,16 @@ SIM_AC_OPTION_STDIO
>> SIM_AC_OPTION_TRACE
>> SIM_AC_OPTION_WARNINGS
>> +AC_MSG_CHECKING(for a known getopt prototype in unistd.h)
>> +AC_CACHE_VAL(sim_cv_decl_getopt_unistd_h,
>> +[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([#include <unistd.h>], [extern
>> int getopt (int, char *const*, const char *);])],
>> +sim_cv_decl_getopt_unistd_h=yes, sim_cv_decl_getopt_unistd_h=no)])
>> +AC_MSG_RESULT($sim_cv_decl_getopt_unistd_h)
>> +if test $sim_cv_decl_getopt_unistd_h = yes; then
>> + AC_DEFINE([HAVE_DECL_GETOPT], 1,
>> + [Is the prototype for getopt in <unistd.h> in the expected
>> format?])
>> +fi
>> +
>> dnl These are unfortunate. They are conditionally called by other
>> sim macros
>> dnl but always used by common/Make-common.in. So we have to subst
>> here even
>> dnl when the rest of the code is in the respective macros. Once we
>> merge the
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/5] sim: Check known getopt definition existence
2022-10-12 17:03 ` Tsukasa OI
@ 2022-10-12 17:08 ` Tom de Vries
2022-10-12 17:20 ` Tom de Vries
0 siblings, 1 reply; 38+ messages in thread
From: Tom de Vries @ 2022-10-12 17:08 UTC (permalink / raw)
To: Tsukasa OI; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 257 bytes --]
On 10/12/22 19:03, Tsukasa OI wrote:
> Adding
> #include <getopt.h>
> after
> #include <sys/types.h>
> might change something but I cannot guarantee whether this change would
> fix the issue.
Ack, I'll try attached patch on the try-buildbot.
Thanks,
- Tom
[-- Attachment #2: 0001-sim-m32c-Add-missing-include-getopt.h-in-main.c.patch --]
[-- Type: text/x-patch, Size: 894 bytes --]
sim/m32c: Add missing include getopt.h in main.c
On buildbot gdb-centos-x86_64 we run into:
...
sim/m32c/main.c: In function ‘main’:
sim/m32c/main.c:143:3: error: implicit declaration of function ‘getopt’ \
[-Werror=implicit-function-declaration]
while ((o = getopt (argc, argv, "tc:vdm:C")) != -1)
^
cc1: all warnings being treated as errors
make[3]: *** [main.o] Error 1
...
Fix this by adding the missing include of getopt.h.
Build on x86_64-linux.
---
sim/m32c/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sim/m32c/main.c b/sim/m32c/main.c
index 958ca27ab2b..4ec30c7c7d0 100644
--- a/sim/m32c/main.c
+++ b/sim/m32c/main.c
@@ -29,6 +29,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include <setjmp.h>
#include <signal.h>
#include <sys/types.h>
+#include <getopt.h>
#ifdef HAVE_SYS_SOCKET_H
#ifdef HAVE_NETINET_IN_H
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/5] sim: Check known getopt definition existence
2022-10-12 17:08 ` Tom de Vries
@ 2022-10-12 17:20 ` Tom de Vries
0 siblings, 0 replies; 38+ messages in thread
From: Tom de Vries @ 2022-10-12 17:20 UTC (permalink / raw)
To: Tsukasa OI; +Cc: gdb-patches
On 10/12/22 19:08, Tom de Vries wrote:
> On 10/12/22 19:03, Tsukasa OI wrote:
>> Adding
>> #include <getopt.h>
>> after
>> #include <sys/types.h>
>> might change something but I cannot guarantee whether this change would
>> fix the issue.
>
> Ack, I'll try attached patch on the try-buildbot.
>
I've tried it out, and it still fails the same:
...
/srv/buildbot/worker/gdb-try-centos-x86_64/gdb-build/sim/../../binutils-gdb/sim/m32c/main.c:
In function ‘main’:
/srv/buildbot/worker/gdb-try-centos-x86_64/gdb-build/sim/../../binutils-gdb/sim/m32c/main.c:144:3:
error: implicit declaration of function ‘getopt’
[-Werror=implicit-function-declaration]
while ((o = getopt (argc, argv, "tc:vdm:C")) != -1)
^
cc1: all warnings being treated as errors
make[3]: *** [main.o] Error 1
make[3]: Leaving directory
`/srv/buildbot/worker/gdb-try-centos-x86_64/gdb-build/sim/m32c'
...
Thanks,
- Tom
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/5] sim: Check known getopt definition existence
2022-10-12 16:28 ` Tom de Vries
2022-10-12 17:03 ` Tsukasa OI
@ 2022-10-13 9:50 ` Tsukasa OI
1 sibling, 0 replies; 38+ messages in thread
From: Tsukasa OI @ 2022-10-13 9:50 UTC (permalink / raw)
To: Tom de Vries; +Cc: Nick Clifton, gdb-patches, Binutils
On 2022/10/13 1:28, Tom de Vries wrote:
> On 10/6/22 08:43, Tsukasa OI via Gdb-patches wrote:
>> Clang generates a warning if there is a function declaration/definition
>> with zero arguments. Such declarations/definitions without a
>> prototype (an
>> argument list) are deprecated forms of indefinite arguments
>> ("-Wdeprecated-non-prototype"). On the default configuration, it
>> causes a
>> build failure (unless "--disable-werror" is specified).
>>
>> include/getopt.h defines some getopt function definitions but one of them
>> has a form "extern int getopt ();". If this form is selected in
>> include/getopt.h, Clang generates a warning and the build fails by
>> default.
>>
>> In really old environments, this getopt definition with no arguments is
>> necessary (because the definition may change between environments).
>> However, this definition is now a cause of problems on modern
>> environments.
>>
>> A good news is, this definition is not always selected (e.g. if used by
>> binutils/*.c). This is because configuration scripts of binutils, gas,
>> gprof and ld tries to find known definition of getopt function is used
>> and
>> defines HAVE_DECL_GETOPT macro. If this macro is defined when
>> getopt.h is
>> included, a good form of getopt is used and Clang won't generate
>> warnings.
>>
>> This commit adds a modified portion of ld/configure.ac to find the known
>> getopt definition. If we could find one (and we *will* in most modern
>> environments), we don't need to rely on the deprecated definition.
>
> I'm guessing this cause the build breakage on buildbot gdb-centos-x86_64 .
>
> https://builder.sourceware.org/buildbot/#/builders/71/builds/1392
>
> Thanks,
> - Tom
Hi Tom,
I finally found a cause. First of all, this is reproduced with
following configurations (examples are selected to minimize time to
reproduce):
- CentOS 7 (x86_64; with GNU libc 2.17)
- "make all-sim" with following configurations (for example):
- --target=m32c-unknown-elf
- --target=rl78-unknown-elf
And it was true that my commit (as you pointed out) was the initial bad
commit but the real cause was much complex than I expected. The reason
I could not initially reproduce the issue was because it required GNU
libc <= 2.25 to reproduce.
An interesting fact is... standard unistd.h on GNU libc <= 2.25 includes
<getopt.h> (with __need_getopt macro defined) but it actually includes
$(srcdir)/include/getopt.h, not getopt.h in GNU libc. Since my change
defined HAVE_DECL_GETOPT to 1 on GNU libc-based environment and
declaration of the getopt function is suppressed, causing an error.
On GNU libc >= 2.26, unistd.h includes <bits/getopt_posix.h> without
defining __need_getopt and that's why this error is suppressed on newer
GNU libc-based environments.
Possible reason why this bug is missed is, there are not so many getopt
callers (many calls getopt_long or getopt_long_only, not getopt).
True getopt callers on Binutils/GDB/GCC are following:
- M32C simulator (affected by my change)
- RL78 simulator (affected by my change)
- gprofng (getopt declaration not checked by gprofng/configure)
... yes, GCC (entirely) and the most of Binutils components do not
depend on getopt function anymore. This fact explains why this bug is
not discovered so long.
The true fix to this is going to be applied to include/getopt.h
(Binutils) to detect this include path on GNU libc <= 2.25.
Aside from this, some sim source files need to be changed (to minimize
problems even further). I'll submit these patchsets soon.
Thanks,
Tsukasa
>
>
>> ---
>> sim/config.h.in | 3 +++
>> sim/configure | 32 ++++++++++++++++++++++++++++++++
>> sim/configure.ac | 10 ++++++++++
>> 3 files changed, 45 insertions(+)
>>
>> diff --git a/sim/config.h.in b/sim/config.h.in
>> index 84c363c0aec..9a94b289e46 100644
>> --- a/sim/config.h.in
>> +++ b/sim/config.h.in
>> @@ -41,6 +41,9 @@
>> /* Define to 1 if you have the `chmod' function. */
>> #undef HAVE_CHMOD
>> +/* Is the prototype for getopt in <unistd.h> in the expected
>> format? */
>> +#undef HAVE_DECL_GETOPT
>> +
>> /* Define to 1 if you have the declaration of `tzname', and to 0 if
>> you don't.
>> */
>> #undef HAVE_DECL_TZNAME
>> diff --git a/sim/configure b/sim/configure
>> index 75d1935df38..dac7f085be1 100755
>> --- a/sim/configure
>> +++ b/sim/configure
>> @@ -16428,6 +16428,38 @@ $as_echo "${WARN_CFLAGS} ${WERROR_CFLAGS}"
>> >&6; }
>> fi
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for a known
>> getopt prototype in unistd.h" >&5
>> +$as_echo_n "checking for a known getopt prototype in unistd.h... "
>> >&6; }
>> +if ${sim_cv_decl_getopt_unistd_h+:} false; then :
>> + $as_echo_n "(cached) " >&6
>> +else
>> + cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>> +/* end confdefs.h. */
>> +#include <unistd.h>
>> +int
>> +main ()
>> +{
>> +extern int getopt (int, char *const*, const char *);
>> + ;
>> + return 0;
>> +}
>> +_ACEOF
>> +if ac_fn_c_try_compile "$LINENO"; then :
>> + sim_cv_decl_getopt_unistd_h=yes
>> +else
>> + sim_cv_decl_getopt_unistd_h=no
>> +fi
>> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>> +fi
>> +
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
>> $sim_cv_decl_getopt_unistd_h" >&5
>> +$as_echo "$sim_cv_decl_getopt_unistd_h" >&6; }
>> +if test $sim_cv_decl_getopt_unistd_h = yes; then
>> +
>> +$as_echo "#define HAVE_DECL_GETOPT 1" >>confdefs.h
>> +
>> +fi
>> +
>> diff --git a/sim/configure.ac b/sim/configure.ac
>> index 66a1020efe0..be0cfdbea32 100644
>> --- a/sim/configure.ac
>> +++ b/sim/configure.ac
>> @@ -177,6 +177,16 @@ SIM_AC_OPTION_STDIO
>> SIM_AC_OPTION_TRACE
>> SIM_AC_OPTION_WARNINGS
>> +AC_MSG_CHECKING(for a known getopt prototype in unistd.h)
>> +AC_CACHE_VAL(sim_cv_decl_getopt_unistd_h,
>> +[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([#include <unistd.h>], [extern
>> int getopt (int, char *const*, const char *);])],
>> +sim_cv_decl_getopt_unistd_h=yes, sim_cv_decl_getopt_unistd_h=no)])
>> +AC_MSG_RESULT($sim_cv_decl_getopt_unistd_h)
>> +if test $sim_cv_decl_getopt_unistd_h = yes; then
>> + AC_DEFINE([HAVE_DECL_GETOPT], 1,
>> + [Is the prototype for getopt in <unistd.h> in the expected
>> format?])
>> +fi
>> +
>> dnl These are unfortunate. They are conditionally called by other
>> sim macros
>> dnl but always used by common/Make-common.in. So we have to subst
>> here even
>> dnl when the rest of the code is in the respective macros. Once we
>> merge the
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/5] sim: Check known getopt definition existence
2022-10-06 6:43 ` [PATCH v3 4/5] sim: Check known getopt definition existence Tsukasa OI
2022-10-12 16:28 ` Tom de Vries
@ 2022-10-23 12:16 ` Mike Frysinger
2022-10-27 2:02 ` Tsukasa OI
1 sibling, 1 reply; 38+ messages in thread
From: Mike Frysinger @ 2022-10-23 12:16 UTC (permalink / raw)
To: Tsukasa OI; +Cc: Andrew Burgess, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]
On 06 Oct 2022 06:43, Tsukasa OI wrote:
> Clang generates a warning if there is a function declaration/definition
> with zero arguments. Such declarations/definitions without a prototype (an
> argument list) are deprecated forms of indefinite arguments
> ("-Wdeprecated-non-prototype"). On the default configuration, it causes a
> build failure (unless "--disable-werror" is specified).
>
> include/getopt.h defines some getopt function definitions but one of them
> has a form "extern int getopt ();". If this form is selected in
> include/getopt.h, Clang generates a warning and the build fails by default.
>
> In really old environments, this getopt definition with no arguments is
> necessary (because the definition may change between environments).
> However, this definition is now a cause of problems on modern environments.
>
> A good news is, this definition is not always selected (e.g. if used by
> binutils/*.c). This is because configuration scripts of binutils, gas,
> gprof and ld tries to find known definition of getopt function is used and
> defines HAVE_DECL_GETOPT macro. If this macro is defined when getopt.h is
> included, a good form of getopt is used and Clang won't generate warnings.
>
> This commit adds a modified portion of ld/configure.ac to find the known
> getopt definition. If we could find one (and we *will* in most modern
> environments), we don't need to rely on the deprecated definition.
> ---
> sim/config.h.in | 3 +++
> sim/configure | 32 ++++++++++++++++++++++++++++++++
> sim/configure.ac | 10 ++++++++++
this logic belongs in m4/sim_ac_platform.m4, not configure.ac
should leave a comment above the code too indicating that this logic is
purely for local getopt.h usage, and is copied from other dirs (e.g. the
binutils files).
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/5] sim: Check known getopt definition existence
2022-10-23 12:16 ` Mike Frysinger
@ 2022-10-27 2:02 ` Tsukasa OI
2023-01-03 3:12 ` Mike Frysinger
0 siblings, 1 reply; 38+ messages in thread
From: Tsukasa OI @ 2022-10-27 2:02 UTC (permalink / raw)
To: Mike Frysinger, gdb-patches
On 2022/10/23 21:16, Mike Frysinger wrote:
> On 06 Oct 2022 06:43, Tsukasa OI wrote:
>> Clang generates a warning if there is a function declaration/definition
>> with zero arguments. Such declarations/definitions without a prototype (an
>> argument list) are deprecated forms of indefinite arguments
>> ("-Wdeprecated-non-prototype"). On the default configuration, it causes a
>> build failure (unless "--disable-werror" is specified).
>>
>> include/getopt.h defines some getopt function definitions but one of them
>> has a form "extern int getopt ();". If this form is selected in
>> include/getopt.h, Clang generates a warning and the build fails by default.
>>
>> In really old environments, this getopt definition with no arguments is
>> necessary (because the definition may change between environments).
>> However, this definition is now a cause of problems on modern environments.
>>
>> A good news is, this definition is not always selected (e.g. if used by
>> binutils/*.c). This is because configuration scripts of binutils, gas,
>> gprof and ld tries to find known definition of getopt function is used and
>> defines HAVE_DECL_GETOPT macro. If this macro is defined when getopt.h is
>> included, a good form of getopt is used and Clang won't generate warnings.
>>
>> This commit adds a modified portion of ld/configure.ac to find the known
>> getopt definition. If we could find one (and we *will* in most modern
>> environments), we don't need to rely on the deprecated definition.
>> ---
>> sim/config.h.in | 3 +++
>> sim/configure | 32 ++++++++++++++++++++++++++++++++
>> sim/configure.ac | 10 ++++++++++
>
> this logic belongs in m4/sim_ac_platform.m4, not configure.ac
>
> should leave a comment above the code too indicating that this logic is
> purely for local getopt.h usage, and is copied from other dirs (e.g. the
> binutils files).
> -mike
OK, I'll submit a patch to move this portion to
sim/m4/sim_ac_platform.m4 with proper commit message.
Thanks,
Tsukasa
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/5] sim: Check known getopt definition existence
2022-10-27 2:02 ` Tsukasa OI
@ 2023-01-03 3:12 ` Mike Frysinger
2023-01-03 8:47 ` Tsukasa OI
0 siblings, 1 reply; 38+ messages in thread
From: Mike Frysinger @ 2023-01-03 3:12 UTC (permalink / raw)
To: Tsukasa OI; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2186 bytes --]
On 27 Oct 2022 11:02, Tsukasa OI wrote:
> On 2022/10/23 21:16, Mike Frysinger wrote:
> > On 06 Oct 2022 06:43, Tsukasa OI wrote:
> >> Clang generates a warning if there is a function declaration/definition
> >> with zero arguments. Such declarations/definitions without a prototype (an
> >> argument list) are deprecated forms of indefinite arguments
> >> ("-Wdeprecated-non-prototype"). On the default configuration, it causes a
> >> build failure (unless "--disable-werror" is specified).
> >>
> >> include/getopt.h defines some getopt function definitions but one of them
> >> has a form "extern int getopt ();". If this form is selected in
> >> include/getopt.h, Clang generates a warning and the build fails by default.
> >>
> >> In really old environments, this getopt definition with no arguments is
> >> necessary (because the definition may change between environments).
> >> However, this definition is now a cause of problems on modern environments.
> >>
> >> A good news is, this definition is not always selected (e.g. if used by
> >> binutils/*.c). This is because configuration scripts of binutils, gas,
> >> gprof and ld tries to find known definition of getopt function is used and
> >> defines HAVE_DECL_GETOPT macro. If this macro is defined when getopt.h is
> >> included, a good form of getopt is used and Clang won't generate warnings.
> >>
> >> This commit adds a modified portion of ld/configure.ac to find the known
> >> getopt definition. If we could find one (and we *will* in most modern
> >> environments), we don't need to rely on the deprecated definition.
> >> ---
> >> sim/config.h.in | 3 +++
> >> sim/configure | 32 ++++++++++++++++++++++++++++++++
> >> sim/configure.ac | 10 ++++++++++
> >
> > this logic belongs in m4/sim_ac_platform.m4, not configure.ac
> >
> > should leave a comment above the code too indicating that this logic is
> > purely for local getopt.h usage, and is copied from other dirs (e.g. the
> > binutils files).
>
> OK, I'll submit a patch to move this portion to
> sim/m4/sim_ac_platform.m4 with proper commit message.
have you had a chance to fix this up yet ?
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/5] sim: Check known getopt definition existence
2023-01-03 3:12 ` Mike Frysinger
@ 2023-01-03 8:47 ` Tsukasa OI
0 siblings, 0 replies; 38+ messages in thread
From: Tsukasa OI @ 2023-01-03 8:47 UTC (permalink / raw)
To: Mike Frysinger; +Cc: gdb-patches
On 2023/01/03 12:12, Mike Frysinger wrote:
> On 27 Oct 2022 11:02, Tsukasa OI wrote:
>> On 2022/10/23 21:16, Mike Frysinger wrote:
>>> On 06 Oct 2022 06:43, Tsukasa OI wrote:
>>>> Clang generates a warning if there is a function declaration/definition
>>>> with zero arguments. Such declarations/definitions without a prototype (an
>>>> argument list) are deprecated forms of indefinite arguments
>>>> ("-Wdeprecated-non-prototype"). On the default configuration, it causes a
>>>> build failure (unless "--disable-werror" is specified).
>>>>
>>>> include/getopt.h defines some getopt function definitions but one of them
>>>> has a form "extern int getopt ();". If this form is selected in
>>>> include/getopt.h, Clang generates a warning and the build fails by default.
>>>>
>>>> In really old environments, this getopt definition with no arguments is
>>>> necessary (because the definition may change between environments).
>>>> However, this definition is now a cause of problems on modern environments.
>>>>
>>>> A good news is, this definition is not always selected (e.g. if used by
>>>> binutils/*.c). This is because configuration scripts of binutils, gas,
>>>> gprof and ld tries to find known definition of getopt function is used and
>>>> defines HAVE_DECL_GETOPT macro. If this macro is defined when getopt.h is
>>>> included, a good form of getopt is used and Clang won't generate warnings.
>>>>
>>>> This commit adds a modified portion of ld/configure.ac to find the known
>>>> getopt definition. If we could find one (and we *will* in most modern
>>>> environments), we don't need to rely on the deprecated definition.
>>>> ---
>>>> sim/config.h.in | 3 +++
>>>> sim/configure | 32 ++++++++++++++++++++++++++++++++
>>>> sim/configure.ac | 10 ++++++++++
>>>
>>> this logic belongs in m4/sim_ac_platform.m4, not configure.ac
>>>
>>> should leave a comment above the code too indicating that this logic is
>>> purely for local getopt.h usage, and is copied from other dirs (e.g. the
>>> binutils files).
>>
>> OK, I'll submit a patch to move this portion to
>> sim/m4/sim_ac_platform.m4 with proper commit message.
>
> have you had a chance to fix this up yet ?
> -mike
I'm really sorry. Due to my poor health condition and upcoming
relocation, it's difficult to keep up with Binutils/GDB until... April
or May. But this is definitely the one I should have done before I get
so busy (and messy).
I'll submit a patch by Jan 6.
Tsukasa
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 5/5] sim: Initialize pbb_br_* by default
2022-10-06 6:43 ` [PATCH v3 0/5] sim: Suppress warnings if built with Clang Tsukasa OI
` (3 preceding siblings ...)
2022-10-06 6:43 ` [PATCH v3 4/5] sim: Check known getopt definition existence Tsukasa OI
@ 2022-10-06 6:43 ` Tsukasa OI
2022-10-11 14:20 ` [PATCH v3 0/5] sim: Suppress warnings if built with Clang Andrew Burgess
2022-10-11 16:40 ` Tom de Vries
6 siblings, 0 replies; 38+ messages in thread
From: Tsukasa OI @ 2022-10-06 6:43 UTC (permalink / raw)
To: Tsukasa OI, Andrew Burgess, Mike Frysinger; +Cc: gdb-patches
On the files generated by sim/common/genmloop.sh, variables pbb_br_type and
pbb_br_npc are declared uninitialized and passed to other functions in some
cases. Despite that those are harmless, they will generate GCC warnings
("-Wmaybe-uninitialized").
This commit ensures that pbb_br_type and pbb_br_npc variables are
initialized to a harmless value.
---
sim/common/genmloop.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sim/common/genmloop.sh b/sim/common/genmloop.sh
index 1bbeb615b05..5f6456d7159 100755
--- a/sim/common/genmloop.sh
+++ b/sim/common/genmloop.sh
@@ -1167,8 +1167,8 @@ void
SEM_PC vpc;
#if WITH_SEM_SWITCH_FULL
/* For communication between cti's and cti-chain. */
- SEM_BRANCH_TYPE pbb_br_type;
- PCADDR pbb_br_npc;
+ SEM_BRANCH_TYPE pbb_br_type = SEM_BRANCH_UNTAKEN;
+ PCADDR pbb_br_npc = 0;
#endif
EOF
@@ -1259,8 +1259,8 @@ void
SEM_PC vpc;
#if WITH_SEM_SWITCH_FAST
/* For communication between cti's and cti-chain. */
- SEM_BRANCH_TYPE pbb_br_type;
- PCADDR pbb_br_npc;
+ SEM_BRANCH_TYPE pbb_br_type = SEM_BRANCH_UNTAKEN;
+ PCADDR pbb_br_npc = 0;
#endif
EOF
--
2.34.1
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 0/5] sim: Suppress warnings if built with Clang
2022-10-06 6:43 ` [PATCH v3 0/5] sim: Suppress warnings if built with Clang Tsukasa OI
` (4 preceding siblings ...)
2022-10-06 6:43 ` [PATCH v3 5/5] sim: Initialize pbb_br_* by default Tsukasa OI
@ 2022-10-11 14:20 ` Andrew Burgess
2022-10-11 16:40 ` Tom de Vries
6 siblings, 0 replies; 38+ messages in thread
From: Andrew Burgess @ 2022-10-11 14:20 UTC (permalink / raw)
To: Tsukasa OI, Tsukasa OI, Mike Frysinger; +Cc: gdb-patches
Tsukasa OI <research_trasio@irq.a4lg.com> writes:
> Hello,
>
> This is the version 2 patchset to suppress Clang compiler warnings
> (causes a build failure due to default -Werror).
>
>
> [Background]
>
> When we build Binutils and GDB with Clang, it causes a build failure due to
> warnings generated by Clang and the default -Werror configuration.
>
> I MOSTLY managed to make ALL ARCHITECTURE ENABLED Binutils and GDB
> -Werror-free on Clang 15.0.0 (note that this does not necessarily mean
> warning-free) and this patchset is a part of it (common simulator parts).
>
> - It still requires -Wno-implicit-function-declaration on the
> LatticeMicro32 and M32R simulators. Except them, most of Binutils / GDB
> components can be built with Clang by default with my tree.
> - I noticed that the assembler is not tested enough (because just with
> --enable-targets=all, it builds only host assembler).
>
> Full Clang 15.0.0 -Werror-free branch in development is available at:
> <https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-dev>
>
> Tested configuration:
> - Ubuntu 22.04.1 LTS (x86_64)
> - LLVM / Clang 15.0.0 (built from source)
> - Configuration examples:
> $srcdir/configure \
> --enable-targets=all \
> --enable-multilib --enable-ld --enable-gold --enable-nls \
> CC=clang CXX=clang++ CCLD=clang CXXLD=clang++ \
> CFLAGS=' -O2 -g -Wno-implicit-function-declaration' \
> CXXFLAGS='-O2 -g -Wno-implicit-function-declaration'
> $srcdir/configure \
> --target=riscv64-unknown-linux-gnu \
> --enable-multilib --enable-ld --enable-gold --enable-nls \
> CC=clang CXX=clang++ CCLD=clang CXXLD=clang++ \
> CFLAGS='-O2 -g' CXXFLAGS='-O2 -g'
>
>
> [About this Patchset]
>
> This patchset contains four five fixes to the files under sim{,/common}.
> Each contains minor fixes to suppress Clang / GCC warnings.
>
> Each change is detailed in the each patch.
>
>
> [Changes: v2 -> v3]
>
> - PATCH 2/5: Rewritten predicate macros from (!!x) to (x != 0).
> (This is based on the feedback by Andrew Burgess).
> - PATCH 3/5: Replaced with a patch by Andrew Burgess
> (slightly modified by me).
> - PATCH 4/5: Fix cached variable from ld_cv_* to sim_cv_*.
> - Removed ChangeLog from commit messages
> (based on the feedback [to another patchset by me] by Bruno Larsen).
>
>
> [Changes: v1 -> v2]
>
> - Commit messages are improved
> - PATCH v1 1/4 is Moved to another patchset
> - PATCH v1 2-4/4 -> PATCH v2 1-3/5
> - New: Check known getopt definition on sim (PATCH v2 4/5)
> This test is already performed on binutils, gas, gprof and ld and this
> commit does the same to sim.
> - New: Initialize pbb_br_* by default (PATCH v2 5/5)
> It did not generate warnings on Clang but GCC creates some warnings
> (although those are not reported as errors).
> This is due to uninitialized pbb_br_* variables and this commit fixes
> the issue by initializing them by a harmless value.
Thanks for the updated patches. I reviewed these and pushed them with a
couple of minor updates, I'll follow up to each patch with the changes I
made.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 0/5] sim: Suppress warnings if built with Clang
2022-10-06 6:43 ` [PATCH v3 0/5] sim: Suppress warnings if built with Clang Tsukasa OI
` (5 preceding siblings ...)
2022-10-11 14:20 ` [PATCH v3 0/5] sim: Suppress warnings if built with Clang Andrew Burgess
@ 2022-10-11 16:40 ` Tom de Vries
2022-10-11 18:02 ` Tsukasa OI
6 siblings, 1 reply; 38+ messages in thread
From: Tom de Vries @ 2022-10-11 16:40 UTC (permalink / raw)
To: Tsukasa OI, Andrew Burgess, Mike Frysinger; +Cc: gdb-patches
On 10/6/22 08:43, Tsukasa OI via Gdb-patches wrote:
> Hello,
>
> This is the version 2 patchset to suppress Clang compiler warnings
> (causes a build failure due to default -Werror).
>
>
Hi,
AFAICT, this causes breakage with the build bots, see f.i.
https://builder.sourceware.org/buildbot/#/builders/74/builds/1357 :
...
CC pk_disklabel.o
../../../binutils-gdb/sim/ppc/corefile.c: In function
‘core_map_find_mapping’:
../../../binutils-gdb/sim/ppc/corefile.c:295:136: error: format ‘%x’
expects argument of type ‘unsigned int’, but argument 4 has type ‘cpu *’
{aka ‘struct _cpu *’} [-Werror=format=]
295 | error("core_find_mapping() - access to unmaped address,
attach a default map to handle this - addr=0x%x nr_bytes=0x%x
processor=0x%x cia=0x%x\n",
|
~^
|
|
|
unsigned int
296 | addr, nr_bytes, processor, cia);
| ~~~~~~~~~
| |
| cpu * {aka struct _cpu *}
cc1: all warnings being treated as errors
make[3]: *** [Makefile:144: corefile.o] Error 1
...
Thanks,
- Tom
> [Background]
>
> When we build Binutils and GDB with Clang, it causes a build failure due to
> warnings generated by Clang and the default -Werror configuration.
>
> I MOSTLY managed to make ALL ARCHITECTURE ENABLED Binutils and GDB
> -Werror-free on Clang 15.0.0 (note that this does not necessarily mean
> warning-free) and this patchset is a part of it (common simulator parts).
>
> - It still requires -Wno-implicit-function-declaration on the
> LatticeMicro32 and M32R simulators. Except them, most of Binutils / GDB
> components can be built with Clang by default with my tree.
> - I noticed that the assembler is not tested enough (because just with
> --enable-targets=all, it builds only host assembler).
>
> Full Clang 15.0.0 -Werror-free branch in development is available at:
> <https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-dev>
>
> Tested configuration:
> - Ubuntu 22.04.1 LTS (x86_64)
> - LLVM / Clang 15.0.0 (built from source)
> - Configuration examples:
> $srcdir/configure \
> --enable-targets=all \
> --enable-multilib --enable-ld --enable-gold --enable-nls \
> CC=clang CXX=clang++ CCLD=clang CXXLD=clang++ \
> CFLAGS=' -O2 -g -Wno-implicit-function-declaration' \
> CXXFLAGS='-O2 -g -Wno-implicit-function-declaration'
> $srcdir/configure \
> --target=riscv64-unknown-linux-gnu \
> --enable-multilib --enable-ld --enable-gold --enable-nls \
> CC=clang CXX=clang++ CCLD=clang CXXLD=clang++ \
> CFLAGS='-O2 -g' CXXFLAGS='-O2 -g'
>
>
> [About this Patchset]
>
> This patchset contains four five fixes to the files under sim{,/common}.
> Each contains minor fixes to suppress Clang / GCC warnings.
>
> Each change is detailed in the each patch.
>
>
> [Changes: v2 -> v3]
>
> - PATCH 2/5: Rewritten predicate macros from (!!x) to (x != 0).
> (This is based on the feedback by Andrew Burgess).
> - PATCH 3/5: Replaced with a patch by Andrew Burgess
> (slightly modified by me).
> - PATCH 4/5: Fix cached variable from ld_cv_* to sim_cv_*.
> - Removed ChangeLog from commit messages
> (based on the feedback [to another patchset by me] by Bruno Larsen).
>
>
> [Changes: v1 -> v2]
>
> - Commit messages are improved
> - PATCH v1 1/4 is Moved to another patchset
> - PATCH v1 2-4/4 -> PATCH v2 1-3/5
> - New: Check known getopt definition on sim (PATCH v2 4/5)
> This test is already performed on binutils, gas, gprof and ld and this
> commit does the same to sim.
> - New: Initialize pbb_br_* by default (PATCH v2 5/5)
> It did not generate warnings on Clang but GCC creates some warnings
> (although those are not reported as errors).
> This is due to uninitialized pbb_br_* variables and this commit fixes
> the issue by initializing them by a harmless value.
>
> Thanks,
> Tsukasa
>
>
>
>
> Tsukasa OI (5):
> sim: Remove self-assignments
> sim: Make WITH_{TRACE,PROFILE}-based macros bool
> sim: Suppress non-literal printf warning
> sim: Check known getopt definition existence
> sim: Initialize pbb_br_* by default
>
> sim/common/genmloop.sh | 8 ++++----
> sim/common/hw-tree.c | 2 --
> sim/common/sim-hw.c | 26 ++++++++++++++++----------
> sim/common/sim-profile.h | 12 ++++++------
> sim/common/sim-trace.h | 4 ++--
> sim/config.h.in | 3 +++
> sim/configure | 32 ++++++++++++++++++++++++++++++++
> sim/configure.ac | 10 ++++++++++
> 8 files changed, 73 insertions(+), 24 deletions(-)
>
>
> base-commit: a13886e2198beb78b81c59839043b021ce6df78a
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 0/5] sim: Suppress warnings if built with Clang
2022-10-11 16:40 ` Tom de Vries
@ 2022-10-11 18:02 ` Tsukasa OI
0 siblings, 0 replies; 38+ messages in thread
From: Tsukasa OI @ 2022-10-11 18:02 UTC (permalink / raw)
To: Tom de Vries, Andrew Burgess; +Cc: gdb-patches
On 2022/10/12 1:40, Tom de Vries wrote:
> On 10/6/22 08:43, Tsukasa OI via Gdb-patches wrote:
>> Hello,
>>
>> This is the version 2 patchset to suppress Clang compiler warnings
>> (causes a build failure due to default -Werror).
>>
>>
>
> Hi,
>
> AFAICT, this causes breakage with the build bots, see f.i.
> https://builder.sourceware.org/buildbot/#/builders/74/builds/1357 :
> ...
> CC pk_disklabel.o
> ../../../binutils-gdb/sim/ppc/corefile.c: In function
> ‘core_map_find_mapping’:
> ../../../binutils-gdb/sim/ppc/corefile.c:295:136: error: format ‘%x’
> expects argument of type ‘unsigned int’, but argument 4 has type ‘cpu *’
> {aka ‘struct _cpu *’} [-Werror=format=]
> 295 | error("core_find_mapping() - access to unmaped address,
> attach a default map to handle this - addr=0x%x nr_bytes=0x%x
> processor=0x%x cia=0x%x\n",
> |
> ~^
> |
> |
> |
> unsigned int
> 296 | addr, nr_bytes, processor, cia);
> | ~~~~~~~~~
> | |
> | cpu * {aka struct _cpu *}
> cc1: all warnings being treated as errors
> make[3]: *** [Makefile:144: corefile.o] Error 1
> ...
>
> Thanks,
> - Tom
OK, I've figured out why.
First of all, ATTRIBUTE_PRINTF should be placed on the declaration
(rather than definition) whenever possible (if there's no declaration,
no need to create one) because it provides better analysis even if a
printf-like function is in the different object file. This is opposite
from what I initially thought (note that I remembered those two words
"definition" and "declaration" incorrectly and be careful when you read
my previous e-mails and commit messages).
Second, when Andrew added ATTRIBUTE_PRINTF on the declaration, it
revealed existing broken printf-like calls and generated some errors,
even on GCC. I'm working on the fix.
Thanks,
Tsukasa
>
>> [Background]
>>
>> When we build Binutils and GDB with Clang, it causes a build failure
>> due to
>> warnings generated by Clang and the default -Werror configuration.
>>
>> I MOSTLY managed to make ALL ARCHITECTURE ENABLED Binutils and GDB
>> -Werror-free on Clang 15.0.0 (note that this does not necessarily mean
>> warning-free) and this patchset is a part of it (common simulator parts).
>>
>> - It still requires -Wno-implicit-function-declaration on the
>> LatticeMicro32 and M32R simulators. Except them, most of
>> Binutils / GDB
>> components can be built with Clang by default with my tree.
>> - I noticed that the assembler is not tested enough (because just with
>> --enable-targets=all, it builds only host assembler).
>>
>> Full Clang 15.0.0 -Werror-free branch in development is available at:
>> <https://github.com/a4lg/binutils-gdb/tree/clang-nowarn-dev>
>>
>> Tested configuration:
>> - Ubuntu 22.04.1 LTS (x86_64)
>> - LLVM / Clang 15.0.0 (built from source)
>> - Configuration examples:
>> $srcdir/configure \
>> --enable-targets=all \
>> --enable-multilib --enable-ld --enable-gold --enable-nls \
>> CC=clang CXX=clang++ CCLD=clang CXXLD=clang++ \
>> CFLAGS=' -O2 -g -Wno-implicit-function-declaration' \
>> CXXFLAGS='-O2 -g -Wno-implicit-function-declaration'
>> $srcdir/configure \
>> --target=riscv64-unknown-linux-gnu \
>> --enable-multilib --enable-ld --enable-gold --enable-nls \
>> CC=clang CXX=clang++ CCLD=clang CXXLD=clang++ \
>> CFLAGS='-O2 -g' CXXFLAGS='-O2 -g'
>>
>>
>> [About this Patchset]
>>
>> This patchset contains four five fixes to the files under sim{,/common}.
>> Each contains minor fixes to suppress Clang / GCC warnings.
>>
>> Each change is detailed in the each patch.
>>
>>
>> [Changes: v2 -> v3]
>>
>> - PATCH 2/5: Rewritten predicate macros from (!!x) to (x != 0).
>> (This is based on the feedback by Andrew Burgess).
>> - PATCH 3/5: Replaced with a patch by Andrew Burgess
>> (slightly modified by me).
>> - PATCH 4/5: Fix cached variable from ld_cv_* to sim_cv_*.
>> - Removed ChangeLog from commit messages
>> (based on the feedback [to another patchset by me] by Bruno Larsen).
>>
>>
>> [Changes: v1 -> v2]
>>
>> - Commit messages are improved
>> - PATCH v1 1/4 is Moved to another patchset
>> - PATCH v1 2-4/4 -> PATCH v2 1-3/5
>> - New: Check known getopt definition on sim (PATCH v2 4/5)
>> This test is already performed on binutils, gas, gprof and ld and
>> this
>> commit does the same to sim.
>> - New: Initialize pbb_br_* by default (PATCH v2 5/5)
>> It did not generate warnings on Clang but GCC creates some warnings
>> (although those are not reported as errors).
>> This is due to uninitialized pbb_br_* variables and this commit
>> fixes
>> the issue by initializing them by a harmless value.
>>
>> Thanks,
>> Tsukasa
>>
>>
>>
>>
>> Tsukasa OI (5):
>> sim: Remove self-assignments
>> sim: Make WITH_{TRACE,PROFILE}-based macros bool
>> sim: Suppress non-literal printf warning
>> sim: Check known getopt definition existence
>> sim: Initialize pbb_br_* by default
>>
>> sim/common/genmloop.sh | 8 ++++----
>> sim/common/hw-tree.c | 2 --
>> sim/common/sim-hw.c | 26 ++++++++++++++++----------
>> sim/common/sim-profile.h | 12 ++++++------
>> sim/common/sim-trace.h | 4 ++--
>> sim/config.h.in | 3 +++
>> sim/configure | 32 ++++++++++++++++++++++++++++++++
>> sim/configure.ac | 10 ++++++++++
>> 8 files changed, 73 insertions(+), 24 deletions(-)
>>
>>
>> base-commit: a13886e2198beb78b81c59839043b021ce6df78a
>
^ permalink raw reply [flat|nested] 38+ messages in thread