[Bug target/77882] [Aarch64] Add 'naked' function attribute

2024-04-08 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77882

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #14 from Nick Desaulniers  ---
Adding a data point from
https://discourse.llvm.org/t/hand-written-in-assembly-in-libc-setjmp-longjmp/73249/9.

It's becoming a code portability issue when clang supports this function
attribute on functions for aarch64 targets but GCC does not.

[Bug tree-optimization/78512] [7 Regression] r242674 miscompiles Linux kernel

2023-09-13 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78512

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #10 from Nick Desaulniers  ---
I'm not super happy that GCC has false-negatives when %p is encountered.  Bugs
do exist outside of the Linux kernel with the usage of %p that could be
flagged.

Clang-18 has recently added -Wno-format-overflow-non-kprintf and
-Wformat-truncation-non-kprintf to emulate this behavior in
https://github.com/llvm/llvm-project/pull/65969, which we will use in the
kernel
https://github.com/ClangBuiltLinux/linux/issues/1923#issuecomment-1718144462.

At the least, I think this behavior wrt. %p should either be documented, or
-Wno-format-overflow-non-kprintf and -Wformat-truncation-non-kprintf
implemented in GCC.

That said, this diagnostic catches real bugs!  Linus turned them off, but we
will work through fixing the instances identified towards the goal of getting
them re-enabled for the Linux kernel.
https://github.com/KSPP/linux/issues/343

[Bug c/111219] -Wformat-truncation false negative with %p modifier

2023-08-28 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111219

--- Comment #2 from Nick Desaulniers  ---
Ah ok that makes sense.

Would it be possible to get that behavior documented on this page?

https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-truncation

We can probably modify clang to match this behavior then.

It's good to know that this was intentional, but too bad that Martin did the
work to change this, but the kernel commit still disabled the diagnostic.

Martin's GCC patch is dated:
Date: Tue Nov 29 21:08:02 2016

Linus' kernel patch is dated:
Date:   Wed Jul 12 19:25:47 2017 -0700

(So this was changed in GCC BEFORE the kernel commit; perhaps Linus was using
an older release at the time. Or perhaps there was something else Linus was
witnessing).

[Bug c/111219] New: -Wformat-truncation false negative with %p modifier

2023-08-28 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111219

Bug ID: 111219
   Summary: -Wformat-truncation false negative with %p modifier
   Product: gcc
   Version: 14.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
  Target Milestone: ---

I noticed that -Wformat-truncation was disabled in the linux kernel.

commit bd664f6b3e37 ("disable new gcc-7.1.1 warnings for now")

I was curious since I was unfamiliar with that flag.  I filed a bug against
clang to look into implementing something similar.

https://github.com/llvm/llvm-project/issues/64871

They extended their existing -Wfortify-source flag instead (*sigh*), but we
noticed now in the Linux kernel that `-Wfortify-source` is flagging a few cases
where kernel devs have added custom format flags for pretty printing oft-used
data structures, which is tripping up this warning, since these format
specifiers are not part of the language standard.

A recent kernel patch looks to re-enable -Wformat-truncation for W=1 kernel
builds.  Nathan noticed that GCC is not warning for the %p related flags,
whereas clang is (with -Wfortify-source).

I don't think GCC's current behavior is intentional?

For example, consider the following code:
```
void foo (void *x) {
char dst [1];
__builtin_snprintf(dst, sizeof(dst), "%p", x);
}
```
Clang-18 (trunk, not yet released, after
https://github.com/llvm/llvm-project/commit/0c9c9dd9a24f9d715d950fef0ac7aae01437af96)
with -Wfortify-source will warn:

```
tmp.c:3:5: warning: 'snprintf' will always be truncated; specified size is 1,
but format string expands to at least 4 [-Wfortify-source]
3 | __builtin_snprintf(dst, sizeof(dst), "%p", x);
  | ^
```

GCC with -Wformat-truncation does not warn, but I think it should.

[Bug c/65213] Extend -Wmissing-declarations to variables [i.e. add -Wmissing-variable-declarations]

2023-08-08 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65213

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #6 from Nick Desaulniers  ---
Thanks for implementing this!

I filed a follow up wrt to how this diagnostic interacts with `register`
storage variables.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110947 PTAL

[Bug c/110947] New: Should -Wmissing-variable-declarations not trigger on register variables?

2023-08-08 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110947

Bug ID: 110947
   Summary: Should -Wmissing-variable-declarations not trigger on
register variables?
   Product: gcc
   Version: 14.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
  Target Milestone: ---

Via:
https://lore.kernel.org/all/CAKwvOd=8kxkD9p+WW-F047ShN=r32slyyfpgzhydw3bxtdd...@mail.gmail.com/

I'm looking to enable -Wmissing-variable-declarations in the Linux kernel
(gcc-14 just gained support for this warning).

In one case I noticed that:

register unsigned long current_stack_pointer asm("rsp");

declared in a header would trigger this:

> no previous declaration for 'current_stack_pointer' 
> [-Wmissing-variable-declarations]

So we could add:

extern unsigned long current_stack_pointer;

before the

register unsigned long current_stack_pointer asm("rsp");

but that seems excessive. Perhaps we can simply not diagnose in that case?

Filed this bug report against clang as well:
https://github.com/llvm/llvm-project/issues/64509

[Bug middle-end/110728] should __attribute__((cleanup())) callback get invoked for indirect edges of asm goto

2023-08-03 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110728

--- Comment #10 from Nick Desaulniers  ---
(In reply to Michael Matz from comment #9)
> That has to do with how we need to (possibly)
> split
> critical edges, which changes label identity, which in turn might actually
> be the thing that's required by the asmgoto.

(Addressing just this point for now.)

I don't think that's a problem for users of asm goto; GCC (and Clang) will
already split critical edges when using asm goto with outputs (thus creating
the case you describe where the label that the asm will jump to has been
synthesized and may differ from the final actual destination).

For example:

```c
void bar (void);

int foo (int y) {
int x;

if (y)
goto end;

asm goto ("# %1 %2":"=r"(x):"i"(&&end)::end);
bar();
return 0;
end:
return x;
}
```

in the above, %1 may not equal %2.

I documented this behavior in clang
https://github.com/llvm/llvm-project/commit/329ef60f3e21fd6845e8e8b0da405cae7eb27267#diff-ec770381d76c859f5f572db789175fe44410a72608f58ad5dbb14335ba56eb97
which will be in the clang-17 release notes once clang-17 ships (currently in
-rc).

I also don't see that being a problem for the Linux kernel at the moment,
though they may need to consider that behavior.

[Bug c/91951] goto + mixed declarations + cleanup attribute considered harmful

2023-07-19 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #3 from Nick Desaulniers  ---
Not sure if it's possible to rename the title of bugs, but a more precise title
for this bug might be:

missed (optimization-level dependent) diagnostic for goto when cleanup
attribute callback is not invoked

[Bug middle-end/110728] should __attribute__((cleanup())) callback get invoked for indirect edges of asm goto

2023-07-19 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110728

--- Comment #7 from Nick Desaulniers  ---
(In reply to Andrew Pinski from comment #1)
> I suspect PR 91951 is the same really.

PR 91951 seems to be about a missing diagnostic dependent on optimization
level.  This bug report is more so a question about codegen.

(In reply to Andrew Pinski from comment #2)
> Techincally x does not go out of scope until the } so I don't see how this
> would act any other way.

Compare the codegen with my example vs replacing the `asm goto (..., l0);` with
`goto l0;`.

With `goto`, the destructor is invoked before taking the backward edge.  With
`asm goto` it is not.  That inconsistency leads me to think there might be a
bug (with `asm goto`).

(In reply to Andrew Pinski from comment #4)
> Oh and computed goto has a similar issue too (PR 37722 which is for C++
> deconstructors but cleanup is the same idea there).

Right, that came up in our discussions, too.  I think the difference may be
that with `asm goto` we have the list of labels of possible jump targets and
thus possible edges. With computed goto ("indirect goto") we don't.  So I think
it makes sense to not run destructors for computed goto, and perhaps the
documentation should be made more explicit about that case (then PR 37722
closed as "Working as intended").

(In reply to John McCall from comment #5)
> > If this gets changed in GCC, I'd be happy to modify clang to match that 
> > updated behavior.
> 
> Policy-wise, I don't think clang would accept a patch making this UB
> (effectively what not calling the destructor/cleanup means) instead of
> ill-formed unless a standards body forced us to.  Not calling the
> destructor/cleanup seems like clearly undesirable behavior, and if we can
> define that away in the compiler with relative ease, we should.

Let me clarify.  If GCC were change behavior of `asm goto` to invoke the
destructor/cleanup function before the backwards edge of `asm goto`, I would
submit a patch to clang to implement that behavior as well.  I think we're
(John and I) in agreement that the destructors should be run.

[Bug middle-end/37722] destructors not called on computed goto

2023-07-19 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37722

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #6 from Nick Desaulniers  ---
(In reply to Jakub Jelinek from comment #5)
> I'm afraid for computed gotos all we can do is just document something
> similar

Right, clang behaves this way, too. And I don't see how we'd change that, for
all of the reasons above.  If the documentation doesn't mention this edge case
with the computed goto extension, it should be revised to do so.

Then this bug can be closed as "Working as intended" IMO.

[Bug c/110728] New: should __attribute__((cleanup())) callback get invoked for indirect edges of asm goto

2023-07-18 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110728

Bug ID: 110728
   Summary: should __attribute__((cleanup())) callback get invoked
for indirect edges of asm goto
   Product: gcc
   Version: 14.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
  Target Milestone: ---

Consider the following test case:

```c
void test4cleanup(int*);
// No errors expected.
void test4(void) {
l0:;
int x __attribute__((cleanup(test4cleanup)));
asm goto("# %l0"l0);
}

```

gcc trunk today generates effectively the following control flow:
```
test4:
.LFB0:
subq$24, %rsp
.L2:
#APP
# .L2
#NO_APP
leaq12(%rsp), %rdi
calltest4cleanup
addq$24, %rsp
ret
```
so if the inline asm blob jumps to `l0`, then the cleanup function is not run.

That seemed surprising, at least when we discussed it on this thread.
https://reviews.llvm.org/D155342#4511887

A fellow Linux kernel dev (who introduced the usage of
__attribute__((cleanup())) (and asm goto, coincidentally) to the kernel) agreed
(on IRC).

For now in clang, we produce a diagnostic since the behavior seems surprising. 
If this gets changed in GCC, I'd be happy to modify clang to match that updated
behavior.

[Bug c/108896] provide "element_count" attribute to give more context to __builtin_dynamic_object_size() and -fsanitize=bounds

2023-05-25 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #46 from Nick Desaulniers  ---
RFC:
https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854

[Bug c/108548] New: gcc asm goto with outputs not implicitly volatile

2023-01-25 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108548

Bug ID: 108548
   Summary: gcc asm goto with outputs not implicitly volatile
   Product: gcc
   Version: 12.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
CC: eli.friedman at gmail dot com, foom at fuhm dot net
  Target Milestone: ---

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile mentions:

> GCC’s optimizers sometimes discard asm statements if they determine there is 
> no need for the output variables. ... Using the volatile qualifier disables 
> these optimizations. asm statements that have no output operands and asm goto 
> statements, are implicitly volatile.

So what about asm goto statements that do have outputs, which GCC recently
gained support for?

Looks like I get different codegen targeting x86 at -O2 with:

```
int foo (void) {
void *x;
asm goto (
"movq   %l1, %0\n\t"
"jmp*%0"
:"=r"(x):::bar);
return 0;
bar:
return 1;
}
```
based on whether the asm goto statement is marked volatile or not.

Should asm goto statements with outputs be implicitly volatile (implying a bug
currently in GCC) or not (implying the documentation could perhaps be updated)?

[Bug c/107385] New: [asm goto] "=r" vs "+r" for outputs along indirect edges

2022-10-24 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107385

Bug ID: 107385
   Summary: [asm goto] "=r" vs "+r" for outputs along indirect
edges
   Product: gcc
   Version: 12.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
CC: isanbard at gmail dot com
  Target Milestone: ---

I was playing around with adding support for outputs along indirect edges of
asm goto in clang.  When I was comparing codegen between various cases, I
noticed a case that looked like a bug to me:

int foo (void) {
int x;
asm goto ("": "=r"(x) ::: bar);
x = 6;
bar:
return x;
}

in gcc 12.2 with `-O2` produces:

foo:
ret

The write to x from the inline asm should be dead along the fallthough or
default path out of the asm goto.  We should have one edge that assigns 6 to
the output.

Changing the output constraint modifier from =r to +r seems to produce the
expected code.

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Extended-Asm mentions

> Be careful when you set output operands inside asm goto only on some possible 
> control flow paths. If you don’t set up the output on given path and never 
> use it on this path, it is okay. Otherwise, you should use ‘+’ constraint 
> modifier meaning that the operand is input and output one. With this modifier 
> you will have the correct values on all possible paths from the asm goto.

I'm not sure if that comment is alluding to this behavior, but it might be nice
to not have to pass in any prior value for output only variables?

[Bug target/65372] -mprofile-kernel undocumented

2022-08-09 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65372

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com,
   ||nemanja.i.ibm at gmail dot com

--- Comment #1 from Nick Desaulniers  ---
I filed a feature request to get this implemented in Clang, since the Linux
kernel uses it for the ppc port.  The immediate request was for documentation
about the change.
https://github.com/llvm/llvm-project/issues/57031

[Bug middle-end/100593] [ELF] -fno-pic: Use GOT to take address of an external default visibility function

2022-04-29 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100593

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #15 from Nick Desaulniers  ---
Any chance we could get -mdirect-extern-access implemented for aarch64? 
Otherwise we're discussing the use of `#pragma GCC visibility push(hidden)` for
use in the linux kernel since it's slightly more portable at the moment.

https://lore.kernel.org/linux-arm-kernel/20220427171241.2426592-3-a...@kernel.org/

[Bug rtl-optimization/104236] asm statements containing %= assembler templates getting merged

2022-01-25 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104236

--- Comment #3 from Nick Desaulniers  ---
Thanks for the feedback. I guess I was expecting these two to be somewhat
equivalent:

void x (int a) {
if (a)
asm("# %0"::"i"(__COUNTER__));
else
asm("# %0"::"i"(__COUNTER__));
}

void y (int a) {
if (a)
asm("# %=");
else
asm("# %=");
}

as was attempted in kernel commit

commit 3d1e236022cc ("objtool: Prevent GCC from merging
annotate_unreachable()")

[Bug bootstrap/104236] New: asm statements containing %= assembler templates getting merged

2022-01-25 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104236

Bug ID: 104236
   Summary: asm statements containing %= assembler templates
getting merged
   Product: gcc
   Version: 12.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: bootstrap
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
CC: bp at alien8 dot de, jpoimboe at redhat dot com,
pinskia at gcc dot gnu.org, segher at kernel dot 
crashing.org
  Target Milestone: ---

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile and
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#AssemblerTemplate mention

> Under certain circumstances, GCC may duplicate (or remove duplicates
> of) your assembly code when optimizing. This can lead to unexpected
> duplicate symbol errors during compilation if your asm code defines
> symbols or labels. Using ‘%=’ (see AssemblerTemplate) may help resolve
> this problem.
>
> ‘%=’ Outputs a number that is unique to each instance of the asm
> statement in the entire compilation. This option is useful when
> creating local labels and referring to them multiple times in a single
> template that generates multiple assembler instructions.

I think I might have found a case where GCC is folding two asm strings that
look identical, but technically contain a %= assembler template.  Perhaps
there's somewhere that checks for parameter equality, and misses checking
whether an asm string contains %= ? (or perhaps %= needs to be expanded
sooner?)

Test files are in:
https://gist.github.com/nickdesaulniers/c2c37edcdbaf3a2deb914d2ea8011a96

I would have expected the inline asm string containing `.Lreachable%=:` (in
media_request_object_complete()) to appear twice in the emitted asm output.  If
I modify one of the two `.Lreachable` asm strings by one character, then I see
both emitted.

[Bug inline-asm/98096] Inconsistent operand numbering for asm goto with in-out operands

2021-12-10 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98096

--- Comment #5 from Nick Desaulniers  ---
While the changes to gcc/stmt.c and the second asm goto statement in
gcc/testsuite/gcc.c-torture/compile/pr98096.c in
https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=72d78655a91bb2f89ac4432cfd6374380d6f9987
LGTM (they fix an actual bug with symbolic references), the changes to
gcc/doc/extend.texi and the first asm goto statement in
gcc/testsuite/gcc.c-torture/compile/pr98096.c seemed to have conflated the
label bug with documenting curious behavior.

Was it ever consider that the behavior should be changed, rather than
documented?

[Bug c/103640] New: asm goto w/ outputs numbering with tied outputs differs from clang

2021-12-09 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103640

Bug ID: 103640
   Summary: asm goto w/ outputs numbering with tied outputs
differs from clang
   Product: gcc
   Version: 12.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
CC: foom at fuhm dot net, segher at gcc dot gnu.org, vmakarov 
at redhat dot com
  Target Milestone: ---

Consider the following input:

void *x(void) {
  void *p = &&foo;
  asm goto ("# %0\n\t# %l1":"+r"(p):::foo);
  foo:;
  return p;
}

%0 refers to the 0th operand `p`.
%l1 refers to the 1st operand `foo`.

This code compiles in Clang, but not GCC.

If you change %l1 to %l2, that will compile in GCC but not clang.

Because +r constraint is being used, I suspect there's a hidden input being
tied to the output.

While we can change this in clang (https://reviews.llvm.org/D115471), I do
think it would be more ergonomic for developers if the order of operands for
the outputs templates matched the order of the asm operands.  Otherwise,
developers need to understand hidden inputs, or use symbolic names like:

asm goto ("# %0\n\t# %l[foo]":"+r"(p):::foo);

I suspect there's not enough users yet of asm goto w/ outputs to notice such a
change, either way.

[Bug libgcc/103034] New: implement multiply with overflow (__muloti4/__mulodi4/__mulosi4)

2021-11-01 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103034

Bug ID: 103034
   Summary: implement multiply with overflow
(__muloti4/__mulodi4/__mulosi4)
   Product: gcc
   Version: 12.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: libgcc
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
  Target Milestone: ---

LLVM's compiler-rt has support for 3 functions for doing multiplication with
overflow checks (__muloti4/__mulodi4/__mulosi4); libcalls to these were emitted
generally when doing multiplication with operands larger than the target's word
size with UBSAN enabled, or via calls to __builtin_mul_overflow().  These seem
to be a pretty potentially a win for code size.

Unfortunately, I had to scale back LLVM's codegen from emitting libcalls to
these; clang doesn't always use LLVM's compiler-rt for many platforms and
targets, so these libcalls are a bit of a portability mess ATM as they do not
exist in libgcc.

For example:
__int128_t a;
int main (void) {
  a = a * a;
  return 0;
}
compiled with -O2 -fsanitize=undefined comes out to about:
gcc-11: 41 instructions
clang-13: 36 instructions
clang-14: 73 instructions

They're generally useful and might be handy for libgcc to implement (then gcc
to implement libcalls to).

The implementations and tests for LLVM can be found in:
compiler-rt/lib/builtins/mulo{tds}i4.c
compiler-rt/lib/builtins/int_mulo_impl.inc
compiler-rt/test/builtins/Unit/mulo{tds}_test.c

[Bug c/91432] gcc -Wimplicit-fallthrough does not warn when fallthrough to break;

2021-07-27 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91432

--- Comment #5 from Nick Desaulniers  ---
> Not warning in this case is a very intentional part of those design decisions.

Can you provide a link to the discussion about this specific case?

Is re-evaluating the decision out of the question?

[Bug c/66425] (void) cast doesn't suppress __attribute__((warn_unused_result))

2021-07-27 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #39 from Nick Desaulniers  ---
Cross referencing an LLVM bug on a similar discussion:
https://bugs.llvm.org/show_bug.cgi?id=51228

[Bug gcov-profile/80223] RFE: Exclude functions from profile instrumentation

2021-06-24 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223

--- Comment #23 from Nick Desaulniers  ---
(In reply to Fangrui Song from comment #18)
> I
> think a similar topic may need to be raided on llvm-dev side as I feel this
> is the tip of the iceberg - more attributes can be similarly leveraged. So,
> how about a llvm-dev discussion?

Sure.
llvm-dev RFC: https://lists.llvm.org/pipermail/llvm-dev/2021-June/151509.html
llvm patch: https://reviews.llvm.org/D104810

[Bug gcov-profile/80223] RFE: Exclude functions from profile instrumentation

2021-06-22 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223

--- Comment #16 from Nick Desaulniers  ---
Clang patch (no_profile -> no_profile_instrument_function):
https://reviews.llvm.org/D104658
Kernel patches v2:
https://lore.kernel.org/lkml/20210621231822.2848305-1-ndesaulni...@google.com/

[Bug gcov-profile/80223] RFE: Exclude functions from profile instrumentation

2021-06-21 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223

--- Comment #15 from Nick Desaulniers  ---
(In reply to Fangrui Song from comment #14)
> Can a no_profile_instrument_function function be inlined into a function
> without the attribute? This may be controversial but I'd argue that it can.
> GCC no_stack_protector behaves this way. no_profile_instrument_function can
> mean that user does not want profiling when the function is called with its
> entity, not via another entity.

I respectfully but strongly disagree. It's surprising to developers when they
ask for no stack protector, or no profiling instrumentation, then get one
anyways.  For long call chains, it's hard for developers to diagnose on their
own which function they called that missed such function attribute.

This reminds me of "what color is your function?"
https://journal.stuffwithstuff.com/2015/02/01/what-color-is-your-function/
As suddenly a developer would need to verify for a no_* attributed function
that they only call no_* attributed functions, or add noinline (which is a big
hammer to all call sites, and games with aliases that have the noinline
attribute are kind of ridiculous).

It's less surprising to prevent inline substitution upon function attribute
mismatch. Then a developer can self diagnose with -Rpass=inline. Either way,
some form of diagnostics would be helpful for these kinds of issues, and has
been requested by Android platform developers working on Zygote.

For no_stack_protector in LLVM, I implemented the rules: upon mismatch, prevent
inline substitution unless the user specified always_inline.  This fixed
suspend/resume bugs in x86 Linux kernels when built with LTO.

Though, I'm happy to revisit that behavior in LLVM; we could add

#define noinline_for_lto __attribute__((__noinline__))

then use that in the Linux kernel instead.

[Bug gcov-profile/80223] RFE: Exclude functions from profile instrumentation

2021-06-21 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223

--- Comment #12 from Nick Desaulniers  ---
Ah, perfect!

commit 1225d6b1134b ("Introduce no_profile_instrument_function attribute")

LGTM: https://godbolt.org/z/779xzndY6

Looks like it landed in GCC 7.1.

Let me change over the attribute identifier in Clang to match, then I'll resend
the kernel patches, and close this out.

[Bug gcov-profile/80223] RFE: Exclude functions from profile instrumentation

2021-06-18 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223

--- Comment #10 from Nick Desaulniers  ---
Link to kernel patches:
https://lore.kernel.org/lkml/20210618233023.1360185-1-ndesaulni...@google.com/

[Bug gcov-profile/80223] RFE: Exclude functions from profile instrumentation

2021-06-17 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223

--- Comment #9 from Nick Desaulniers  ---
(In reply to Fangrui Song from comment #8)
> I am thinking of __attribute__((no_profile)).
> 
> In Clang,
> -fprofile-generate(-fcs-profile-generate)/-fprofile-instr-generate/-fprofile-
> arcs are all different. It will make sense to have a attribute disabling all
> such profiling related features.
> 
> I am not sure an umbrella __attribute__((no_instrument_function)) is
> suitable.
> The Linux kernel wanting noinstr to exclude -fprofile-* is a very specific
> characteristic, not suitable for other applications.

Sure, though now we'll need to add use of that to the Linux kernel. v2:
https://reviews.llvm.org/D104475.

[Bug gcov-profile/80223] RFE: Exclude functions from profile instrumentation

2021-06-14 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223

Nick Desaulniers  changed:

   What|Removed |Added

 CC||elver at google dot com,
   ||isanbard at gmail dot com,
   ||kees at outflux dot net,
   ||maskray at google dot com,
   ||ndesaulniers at google dot com

--- Comment #6 from Nick Desaulniers  ---
We had a request for something like this today on LKML, see the thread.
https://lore.kernel.org/lkml/cakwvodmpti93n2l0_yqkrzldmpxzror7zggszonyaw2pgsh...@mail.gmail.com/

And more specific use case:
https://lore.kernel.org/lkml/20210614190700.gf68...@worktop.programming.kicks-ass.net/

I've implemented this in LLVM; no_instrument_function function attribute in C
can be used to disable coverage of -fprofile-generate (instrumentation based
profiling; "PGO") and -fprofile-arcs (coverage; "GCOV").

PGO: https://reviews.llvm.org/D104253
GCOV: https://reviews.llvm.org/D104257

Inlining is a good point and something I'll need to check; generally when
caller's and callee's function attributes don't match, we block inline
substitution (though we permit it for always_inline; developer be damned).

One question Fangrui had made was whether no_instrument_function is the
appropriate function attribute to re-use.
https://reviews.llvm.org/D104253#2817695

It looks like both -finstrument-functions and -pg are affected by attribute
no_instrument_function; I decided to reuse no_instrument_function in LLVM
because:
1. it already exists; implementation is barely more than 1 LoC.
2. it already affects code gen of 2 different flags.
3. its name perfectly describes developer intent.
4. the Linux kernel is already wired up to make use of no_instrument_function
attribute (though the kernel's configuration step (KConfig) will need changes
to detect support for this issue probably).

I haven't landed the changes in LLVM yet, and don't particularly care what the
attribute used ultimately is even if that means revisting our approach in LLVM.

But without a solution to this problem, it's likely to block PGO and regress
GCOV for x86 Linux kernels.

[Bug tree-optimization/100363] gcc generating wider load/store than warranted at -O3

2021-05-05 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100363

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #20 from Nick Desaulniers  ---
(In reply to Alexander Monakov from comment #7)
>  Most likely the issue is that sout/sfrom are misaligned at runtime, while 
> the > vectorized code somewhere relies on them being sufficiently aligned for 
> a 'short'.
> It is unsafe to dereference a misaligned pointer. The pointed-to-type must
> have reduced alignment:

C 6.3.2.3p7 (N1548) says:

A pointer to an object type may be converted to a pointer to a
different object type. If the resulting pointer is not correctly
aligned) for the referenced type, the behavior is undefined.


===

We're working on adding diagnostics and UBSAN checks for these.  Perhaps with
those in place, we'd be able to spot such a case in the kernel's initramfs
decompression code.

[Bug c/98826] New: [gcc vs g++] qualified type of members of anonymous struct

2021-01-25 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98826

Bug ID: 98826
   Summary: [gcc vs g++] qualified type of members of anonymous
struct
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
CC: richard-gccbugzilla at metafoo dot co.uk
  Target Milestone: ---

Consider the following code:

struct dummy {
  const struct {
int a;
volatile struct { int b; };
  };
  int c;
};

extern void fn(const int *);
extern void fn2(volatile int *);

void test(struct dummy *a) {
  fn(&a->b);
  fn2(&a->b);
  a->a = a->c;
}

I noticed that g++ does not produce any diagnostics for the above, but gcc will
produce 3 diagnostics:

bar.c: In function ‘test’:
bar.c:13:6: warning: passing argument 1 of ‘fn’ discards ‘volatile’ qualifier
from pointer target type [-Wdiscarded-qualifiers]
   13 |   fn(&a->b);
  |  ^
bar.c:9:16: note: expected ‘const int *’ but argument is of type ‘const
volatile int *’
9 | extern void fn(const int *);
  |^~~
bar.c:14:7: warning: passing argument 1 of ‘fn2’ discards ‘const’ qualifier
from pointer target type [-Wdiscarded-qualifiers]
   14 |   fn2(&a->b);
  |   ^
bar.c:10:17: note: expected ‘volatile int *’ but argument is of type ‘const
volatile int *’
   10 | extern void fn2(volatile int *);
  | ^~
bar.c:15:8: error: assignment of member ‘a’ in read-only object
   15 |   a->a = a->c;
  |^

So it seems that the members of the anonymous struct are inheriting the
qualifications of the containing anonymous struct? This also seems to apply
recursively for nested anonymous structs.

Is this an intentional difference between C and C++?

[Bug c/94722] implement __attribute__((no_stack_protector)) function attribute

2020-12-17 Thread ndesaulniers at google dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94722

--- Comment #10 from Nick Desaulniers  ---
(In reply to Jakub Jelinek from comment #9)
> I've said in that thread that I don't really like disabling the inlining, if
> we wanted to make sure everything is stack protected, we'd need to disable
> all the inlining no matter whether the attribute is there or not, because
> inlining by definition removes the stack protector checks, it is only tested
> on function boundaries, not on inline function boundaries.
> The user has the option to add noinline when he wants.

I'm not against reverting my recent change to LLVM's inlining behavior
(https://reviews.llvm.org/D91816, and the follow up to the docs
https://reviews.llvm.org/D93422).  Would be less work to do for inlining
decisions.

In the kernel, we could introduce a macro that's "noinline_for_lto" or
"noinline_due_to_stackprotectors" or such; this specific case only comes about
due to LTO between TUs with -fstack-protector* and -fno-stack-protector and
inlining between them.

Though, I'm still concerned about local cases (forgetting LTO for a moment)
where folks use __attribute__((no_stack_protector)) will basically have to
remember to additionally use __attribute__((noinline)) lest they hit the same
kind of bugs within a TU.  LLVM's current behavior "helps" developers avoid
such pitfalls; otherwise developers will have to be able to diagnose on their
own when such a case arises such that it causes bugs.

[Bug c/96907] [docs] document builtins for fputc and putc

2020-09-10 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96907

--- Comment #5 from Nick Desaulniers  ---
No preferences either way.  I was just comparing differences between compilers
for __has_builtin and noticed GCC returns true for those two.  Is that ok?

[Bug c/96907] New: [docs] document builtins for fputc and putc

2020-09-02 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96907

Bug ID: 96907
   Summary: [docs] document builtins for fputc and putc
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
CC: rv at rasmusvillemoes dot dk, segher at kernel dot 
crashing.org
  Target Milestone: ---

In https://reviews.llvm.org/D86508, I'm going through and working on improving
Clang's compatibility with GCC in terms of supported compiler __builtin_
functions.  I noticed that __has_builtin works with putc, __builtin_putc,
fputc, and __builtin_fputc in GCC.  I think putc and fputc could be added to
this page. https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html

[Bug c/95588] No warning in -Wformat for narrowing formats

2020-06-09 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95588

--- Comment #1 from Nick Desaulniers  ---
In https://bugs.llvm.org/show_bug.cgi?id=41467#c4, the code owner for Clang
seems to indicate that we could move the warnings for narrowing prints (ie.
printing an `int` with `%hh`) to a new warning flag within the -Wformat group
(ie. -Wformat-pedantic).

I'm mostly concerned with the case where the value being printed is either
obviously or possibly too large for the print format specifier, ex.

```
#include 
#include 
void foo(void) {
printf("%hd\n", INT_MAX);
}
```
built with -Wformat.

I'm curious if this is indeed something GCC should warn about, and if so,
should it be part of -Wformat? (I suspect yes for both, or would prefer it to
be yes for both, and then to fix all instances in the Linux kernel caught by
this, but would prefer to collaborate on this).

[Bug c/91432] gcc -Wimplicit-fallthrough does not warn when fallthrough to break;

2020-06-04 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91432

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #3 from Nick Desaulniers  ---
Isn't this still an implicit fallthrough, though?  It may not "be a bug," but
that's why warnings are not errors; they may not result in "bugs."

The issue is that Clang will warn in this case, so developers get confused
about writing compiler-portable code with intentional fallthrough vs being
warned about unintentional fallthrough.

Why shouldn't developers annotate that the implicit fallthrough from `case 1`
to `default` was intentional, making it explicit?

[Bug target/94986] missing diagnostic on ARM thumb2 compilation with -pg when using r7 in inline asm

2020-06-03 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94986

--- Comment #4 from Nick Desaulniers  ---
(In reply to nsz from comment #2)
> on arm the -pg abi is
> 
> func:
>   push {lr}
>   bl _gnu_mcount_nc
>   ...
> 
> so no frame pointer is involved, -pg implying
> -fno-omit-frame-pointer is a historical mistake i think
> (because some targets required fp for -pg, but most don't).
> 
> ideally r7 clobber would just work with -pg -fomit-frame-pointer.
> the alloca problem is a separate issue (that r7 clobber may not
> work with alloca).

Should GCC change this for aaarch32 then (rather than closing the bug)?

[Bug c/94722] implement __attribute__((no_stack_protector)) function attribute

2020-04-22 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94722

--- Comment #3 from Nick Desaulniers  ---
Ah, sorry, just was sent:
https://lore.kernel.org/lkml/20200316180303.GR2156@tucnak/ which is also
relevant regarding naming.  Still happy to buy the beers though.

[Bug c/94722] implement __attribute__((no_stack_protector)) function attribute

2020-04-22 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94722

--- Comment #2 from Nick Desaulniers  ---
Also note this post in the thread [0]:
https://lore.kernel.org/lkml/20200422192113.gg26...@zn.tnic/T/#m88641fe74bdffe7beaa925dfe2d8146a08a47bac,
also
https://lore.kernel.org/lkml/20200422192113.gg26...@zn.tnic/T/#m691ff07f537102f1f5f63d4b65405b44f15956cc

"As we determined upthread (and the reason why we even still have this 
thread): the optimize attribute (and pragma) reset flags from the command 
line (the case in point was -fno-omit-frame-pointer).  So, that's not a
solution for now."

"As you will discover upthread that was tried with GCC and found 
insufficient, as GCC is a bit surprising with optimize attributes: it 
resets every -f option from the command line and applies only the ones 
from the attributes.  Including a potential -fno-omit-frame-pointer, 
causing all kinds of itches :)"

We have the ability to work around differences in attribute identifiers easily
in the Linux kernel, so the difference in naming while suboptimal, isn't
painful for the Linux kernel in practice.  Though I will buy another beer for
the developer if it happens to match Clang's naming. :)

[Bug c/94722] New: implement __attribute__((no_stack_protector)) function attribute

2020-04-22 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94722

Bug ID: 94722
   Summary: implement __attribute__((no_stack_protector)) function
attribute
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
CC: jakub at redhat dot com, mliska at suse dot cz
  Target Milestone: ---

There's a couple of places in the Linux kernel where the placement of stack
protector guards causes problems for functions that do some tricky things. 
We'd like to have the ability to keep -fstack-protector* protections throughout
the kernel, but have finer grain resolution to disable the placement and
checking of stack guards on a per function granularity.

clang-8 added support for the function attribute no_stack_protector.
https://clang.llvm.org/docs/AttributeReference.html#no-stack-protector

With this feature implemented, we could have a more portable solution for the
kernel.

Two examples in the kernel where we could make use of this are
[0] https://lore.kernel.org/lkml/20200422192113.gg26...@zn.tnic/T/#t (call to
cpu_startup_entry() in start_secondary() in arch/x86/kernel/smpboot.c after
calls to boot_init_stack_canary() which has modified the stack guard).
[1]
https://lore.kernel.org/lkml/20180621162324.36656-1-ndesaulni...@google.com/
(custom calling conventions)

[0] was worked around with an empty asm statement and a descriptive comment.
[1] was worked around with `extern inline` with gnu_inline semantics.

I would prefer to use a function attribute for both cases.

[Bug target/92424] [aarch64] Broken code with -fpatchable-function-entry and BTI

2020-01-06 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92424

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #2 from Nick Desaulniers  ---
https://reviews.llvm.org/D72215 is the discussion on the interaction between
the two in LLVM, FWIW.

[Bug preprocessor/91961] __has_attribute expands macro argument

2019-10-02 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91961

--- Comment #4 from Nick Desaulniers  ---
Thanks for the report.  I noticed we had
https://bugs.llvm.org/show_bug.cgi?id=38653 on file, so I cc'ed Clang folks who
might have some thoughts.

[Bug c/91206] -Wformat doesn't warn for %hd with char parameter

2019-08-23 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91206

Nick Desaulniers  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |WONTFIX

--- Comment #4 from Nick Desaulniers  ---
Thanks for the feedback, in https://reviews.llvm.org/rL369791, Nathan made
[unsigned] char -> [unsigned]short warn only for -Wformat-pedantic, not
-Wformat.

[Bug c/91206] -Wformat doesn't warn for %hd with char parameter

2019-07-18 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91206

--- Comment #2 from Nick Desaulniers  ---
Ah, I did file a bug for this in LLVM's issue tracker:
https://bugs.llvm.org/show_bug.cgi?id=41467

[Bug c/91206] New: -Wformat doesn't warn for %hd with char parameter

2019-07-18 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91206

Bug ID: 91206
   Summary: -Wformat doesn't warn for %hd with char parameter
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
  Target Milestone: ---

The linux kernel currently disables -Wformat when built with Clang (see
scripts/Makefile.extrawarn).

When looking into why the warning was disabled, I noticed that Clang and GCC
differ slightly in behavior, which leads to a warning spew when building the
kernel with Clang.

Testing out the specific cases of using different size types with different
format flags, I think I found the difference.

It seems that both compilers warn for modifiers to %d (%hhd, %hd, %ld, %lld)
when passed smaller parameters.

Specifically, it seems that GCC does not warn in the case:

char x = 0;
printf("%hd", x);

with -Wformat, but Clang does, and there are a lot of cases in the Linux kernel
that print char's via %hd.

Specifically, Clang would warn for the above:
warning: format specifies type 'short' but the argument has type 'char'
[-Wformat]

printf("%hd", x);

~~~   ^

%hhd

I think GCC should warn in this case as well, and we should clean up the issues
in the Linux kernel, but today it's hard to get patches accepted without having
to explain this subtly to kernel developers.

But if we disagree, I'm happy to collect more feedback for the Clang developers
if maybe Clang should not be warning for this case?

See also:
https://godbolt.org/z/qeuFoh
https://github.com/ClangBuiltLinux/linux/issues/378

[Bug c/66970] Add __has_builtin() macro

2019-07-10 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66970

--- Comment #15 from Nick Desaulniers  ---
Any progress Martin?  Just to keep beating the dead horse...

Forgetting other compilers for a minute, __has_builtin allows for feature
detection which is much better than compiler version checks which are brittle
and error prone. As new builtins get added over time, their existence can be
checked in a cleaner way with __has_builtin.

[Bug c/87435] "Duplicate const" warning NOT emitted from typedef in -std=c90

2018-09-27 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87435

Nick Desaulniers  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |INVALID

--- Comment #2 from Nick Desaulniers  ---
Ok, looks like this can be a -pedantic warning.

[Bug c/80868] "Duplicate const" warning emitted in `const typeof(foo) bar;`

2018-09-25 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80868

--- Comment #7 from Nick Desaulniers  ---
Forked: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87435

[Bug c/87435] New: "Duplicate const" warning NOT emitted from typedef in -std=c90

2018-09-25 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87435

Bug ID: 87435
   Summary: "Duplicate const" warning NOT emitted from typedef in
-std=c90
   Product: gcc
   Version: 8.2.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
  Target Milestone: ---

Forked from https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80868.

Test case: https://godbolt.org/z/jjstcx

typedef const int t;
const t x;

It seems that for -std=c90 (non pedantic, non GNU), GCC does not warn for a
duplicate type specifier.  That seems to violate C90 6.5.3 constraints: "The
same type qualifier shall not appear more than once in the same specifier-list
or qualifier-list, either directly or via one or more typedefs."

I found this while trying to match GCC's behavior for warning about duplicate
type qualifiers from `typeof` expressions in Clang:
https://reviews.llvm.org/D52248#1243898

[Bug c/80868] "Duplicate const" warning emitted in `const typeof(foo) bar;`

2018-09-24 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80868

--- Comment #5 from Nick Desaulniers  ---
Oh, note in the typedef case:

typedef const int t;
const t x;

It seems that for -std=c89 (non pedantic, non GNU), GCC does not warn.  That
seems to violate C90 6.5.3 constraints: "The same type qualifier shall not
appear more than once in the same specifier-list or qualifier-list, either
directly or via one or more typedefs."

[Bug c/80868] "Duplicate const" warning emitted in `const typeof(foo) bar;`

2018-09-24 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80868

--- Comment #4 from Nick Desaulniers  ---
We can close this bug.  LLVM will match GCC here:
https://reviews.llvm.org/D52248

[Bug middle-end/58670] asm goto miscompilation

2018-09-07 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #10 from Nick Desaulniers  ---
Specifically, it seems this was fixed in the gcc-4.8.2 timeframe.  This shows
no issue in gcc-4.8.2, but the issue from comment #5 in gcc-4.8.1.  It seems
the Linux kernel works around this with a macro called `asm_volatile_goto` that
simply inserts an empty asm block `asm("");` after an `asm goto`, which even in
this reproducer fixes the problem.

https://godbolt.org/z/ZUDRnU

[Bug c/86765] New: invalid -Wmisleading-indentation for double macro expansion with pragmas

2018-07-31 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86765

Bug ID: 86765
   Summary: invalid -Wmisleading-indentation for double macro
expansion with pragmas
   Product: gcc
   Version: 8.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
  Target Milestone: ---

Developing a patch for the Linux kernel, I hit a -Wmisleading-indentation
warning for a particular configuration in gcc 6, 7, and 8.

This is the paired down test case: https://godbolt.org/g/b5dsqH

Looking at the output, the warning is technically correct (puts() was called
unconditionally), but I don't think the compiled code is.  Compared to clang,
clang compiles the conditional check (as expected), but gcc does not.

The kernel patch is trying to add compiler specific pragma's to ignore
clang-7's -Wreturn-stack-address.  We could be better about not emitted
_Pragma's for push/pop if we're not clang, but this still seems like a
miscompilation from gcc to me.

Is there a special rule about GNU statement expressions [0] that I'm missing?

[0] https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html

[Bug target/85927] ud2 instruction generated starting with gcc 8

2018-05-25 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85927

--- Comment #4 from Nick Desaulniers  ---
Thanks for the clarification.

[Bug target/85927] ud2 instruction generated starting with gcc 8

2018-05-25 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85927

--- Comment #2 from Nick Desaulniers  ---
Sorry, probably:

__attribute__((naked))
unsigned long save_flags4(void) {
asm volatile("pushf; pop %rax;ret;");
}

is a better example:

 :
   0:   9c  pushfq 
   1:   58  pop%rax
   2:   c3  retq   
   3:   0f 0b   ud2 

(gcc-7 replaces ud2 with another ret, but I guess both should work in
practice).

[Bug c/85927] New: ud2 instruction generated starting with gcc 8

2018-05-25 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85927

Bug ID: 85927
   Summary: ud2 instruction generated starting with gcc 8
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: ndesaulniers at google dot com
  Target Milestone: ---

From: https://lkml.org/lkml/2018/5/25/636

It seems that:

__attribute__((naked))
unsigned long save_flags(void) {
  unsigned long flags;
  asm volatile("pushf; pop %0"
   : "=rm" (flags)
   :
   : "memory");
  return flags;
}

compiled with:

$ gcc-8 temp.c -c -fstack-protector-strong -O3

generates:

$ objdump -d temp.o

temp.o: file format elf64-x86-64


Disassembly of section .text:

 :
   0:   9c  pushfq 
   1:   58  pop%rax
   2:   0f 0b   ud2 

Note that `ud2` is an explicit invalid opcode:
https://x86.puri.sm/html/file_module_x86_id_318.html

>From playing around with this in godbolt, this seems to have regressed from
gcc-7, which generates:

 :
   0:   9c  pushfq 
   1:   58  pop%rax
   2:   c3  retq

(but also gcc-7 warns:
temp.c:10:1: warning: ‘naked’ attribute directive ignored [-Wattributes]
).

[Bug c/80868] "Duplicate const" warning emitted in `const typeof(foo) bar;`

2017-05-23 Thread ndesaulniers at google dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80868

Nick Desaulniers  changed:

   What|Removed |Added

 CC||ndesaulniers at google dot com

--- Comment #1 from Nick Desaulniers  ---
Other discussions:

LKML: https://patchwork.kernel.org/patch/9693821/
Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=723720
LLVM: https://bugs.llvm.org/show_bug.cgi?id=32985