[Bug c/94382] New: conflicting function types should show more context
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94382
Bug ID: 94382
Summary: conflicting function types should show more context
Product: gcc
Version: 9.3.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: matthew at wil dot cx
Target Milestone: ---
The diagnostic would be better if it showed the entire function prototype:
../fs/iomap/apply.c:13:1: error: conflicting types for ‘__iomap_apply’
13 | __iomap_apply(struct inode *inode, loff_t pos, loff_t length, unsigned
flags,
| ^
In file included from ../fs/iomap/apply.c:9:
../include/linux/iomap.h:152:1: note: previous declaration of ‘__iomap_apply’
was here
152 | __iomap_apply(struct inode *inode, loff_t pos, loff_t length, unsigned
flags,
| ^
$ grep -A3 __iomap_apply include/linux/iomap.h fs/iomap/apply.c
include/linux/iomap.h:__iomap_apply(struct inode *inode, loff_t pos, loff_t
length, unsigned flags,
include/linux/iomap.h- const struct iomap_ops *ops, iomap_actor_t
actor,
include/linux/iomap.h- struct iomap *iomap, struct iomap *srcmap);
include/linux/iomap.h-
--
include/linux/iomap.h: length = __iomap_apply(inode, pos, length, flags, ops,
actor,
include/linux/iomap.h- &iomap, &srcmap);
include/linux/iomap.h- /*
include/linux/iomap.h- * Now that we have guaranteed that the space
allocation will succeed,
--
fs/iomap/apply.c:__iomap_apply(struct inode *inode, loff_t pos, loff_t length,
unsigned flags,
fs/iomap/apply.c- struct iomap *iomap, struct iomap *srcmap,
fs/iomap/apply.c- const struct iomap_ops *ops, iomap_actor_t
actor)
fs/iomap/apply.c-{
The bug in my code is fairly obvious, but the diagnostic doesn't show the part
of the function signature that doesn't match.
[Bug middle-end/32911] Function __attribute__ ((idempotent))
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=32911
Matthew Wilcox changed:
What|Removed |Added
CC||matthew at wil dot cx
--- Comment #6 from Matthew Wilcox ---
I actually have a use for a real idempotent function, that is f(f(x)) == f(x).
I think it's different from the behaviour wanted here which is more of an
initialiser functionality.
The concrete example is Linux's compound_head() macro which takes a pointer to
a struct page and returns a pointer to a (potentially different) struct page.
A page is its own head, and calls to compound_head can be buried inside macros.
So I'd like gcc to know that given this program:
struct page *compound_head(struct page *) __attribute__((idempotent));
int g(struct page *page)
{
struct page *head = compound_head(page);
return page->refcount;
}
int f(struct page *page)
{
struct page *head = compound_head(page);
return g(head);
}
if it inlines g() into f(), it only needs to make one call to compound_head().
[Bug middle-end/88518] New: Function call defeats -Wuninitialized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88518
Bug ID: 88518
Summary: Function call defeats -Wuninitialized
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: middle-end
Assignee: unassigned at gcc dot gnu.org
Reporter: matthew at wil dot cx
Target Milestone: ---
void g(unsigned long *);
void h(void);
void f(void)
{
unsigned long i;
h();
i++;
g(&i);
}
$ gcc -Wall -O2 -c test5.c -o /dev/null
reports no warning. If the order of 'i++' and 'h()' are reversed, GCC emits a
warning as expected.
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/8/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 8.2.0-9'
--with-bugurl=file:///usr/share/doc/gcc-8/README.Bugs
--enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr
--with-gcc-major-version-only --program-suffix=-8
--program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie
--with-system-zlib --with-target-system-zlib --enable-objc-gc=auto
--enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64
--with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
--enable-offload-targets=nvptx-none --without-cuda-driver
--enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu
--target=x86_64-linux-gnu
Thread model: posix
gcc version 8.2.0 (Debian 8.2.0-9)
[Bug middle-end/88518] Function call defeats -Wuninitialized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88518
--- Comment #2 from Matthew Wilcox ---
Thanks! What I actually want to do is annotate g() to the effect that it reads
the pointed-to variable before it writes it. IOW, I want to write something
like:
void g(unsigned long __attribute__((read_before_write)) *p);
void h(void);
void f(void)
{
unsigned long i;
h();
g(&i);
}
and have gcc emit a warning that i is used uninitialised. I tried adding i = i
prior to the call of g() through macro trickery, but was surprised that it
didn't trigger, and simplified down to this test-case.
Any chance I could get that attribute or something like it?
[Bug tree-optimization/83377] New: Missed optimization (x86): Bit operations should be converted to arithmetic
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83377
Bug ID: 83377
Summary: Missed optimization (x86): Bit operations should be
converted to arithmetic
Product: gcc
Version: 7.2.1
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: matthew at wil dot cx
Target Milestone: ---
GCC fails to optimise
if (x & 2) y = (x &~ 2)
into
if (x & 2) y = (x - 2)
in cases where that would be advantageous.
Here's one example, which I'm sure will upset the C pedants, but is relatively
common in codebases which use typed pointers based on the lower bits.
int a(void *);
struct s { struct s *s; };
int b(void *x) {
void *p = x;
if ((unsigned long)x & 2)
p = ((struct s *)((unsigned long)x & ~2UL))->s;
return a(p);
}
int c(void *x) {
void *p = x;
if ((unsigned long)x & 2)
p = ((struct s *)((unsigned long)x - 2))->s;
return a(p);
}
On x86, the difference between the assembly output is clear; function c is
smaller than function b:
:
0: 40 f6 c7 02 test $0x2,%dil
4: 74 07 je d
6: 48 83 e7 fd and$0xfffd,%rdi
a: 48 8b 3fmov(%rdi),%rdi
d: e9 00 00 00 00 jmpq 12
12: 0f 1f 40 00 nopl 0x0(%rax)
16: 66 2e 0f 1f 84 00 00nopw %cs:0x0(%rax,%rax,1)
1d: 00 00 00
0020 :
20: 40 f6 c7 02 test $0x2,%dil
24: 74 04 je 2a
26: 48 8b 7f fe mov-0x2(%rdi),%rdi
2a: e9 00 00 00 00 jmpq 2f
This is true with both -O3 and -O2. I have other functions where the savings
are greater (two insns and seven bytes), but the root cause is the same; a
failure to optimise an AND into a SUB (which can then be fused with a load)
I'm filing this one under tree-optimisation rather than RTL, because I think
it's a common feature in CPUs to have load (reg + offset), and relatively
uncommon (in fact I don't know of one) to have load (reg & mask). I'm sure
some CPUs don't even have (reg + offset) addressing modes, but they wouldn't be
harmed by such an optimisation.
[Bug c/83653] New: GCC fails to remove a can't-happen call on ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83653 Bug ID: 83653 Summary: GCC fails to remove a can't-happen call on ia64 Product: gcc Version: 6.4.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: matthew at wil dot cx Target Milestone: --- Created attachment 43009 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43009&action=edit Gzipped preprocessed source This is an excerpt from the Linux kernel, with some patches that I'm preparing to go in. The 0day build-bot reports a problem with an undefined __bad_increment_for_ia64_fetch_and_add. I can reproduce the problem by taking the attached preprocessed source and compiling it with gcc -O2 -o shmem.o -c shmem.i and then running nm shmem.o |grep __bad Manually inlining page_ref_sub() in the included test-case makes the problem go away, as does simply deleting the call to page_ref_sub() (at line 50756). Command line: gcc -Wp,-MD,mm/.shmem.o.d -nostdinc -isystem /usr/lib/gcc/ia64-linux-gnu/4.6/include -I./arch/ia64/include -I./arch/ia64/include/generated -I./include -I./arch/ia64/include/uapi -I./arch/ia64/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -D__KERNEL__ -DHAVE_WORKING_TEXT_ALIGN -DHAVE_MODEL_SMALL_ATTRIBUTE -DHAVE_SERIALIZE_DIRECTIVE -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -Werror-implicit-function-declaration -Wno-format-security -std=gnu89 -Wno-unused-but-set-variable -fno-PIE -pipe -ffixed-r13 -mfixed-range=f12-f15,f32-f127 -falign-functions=32 -frename-registers -fno-optimize-sibling-calls -fno-delete-null-pointer-checks -O2 -DCC_HAVE_ASM_GOTO -Wframe-larger-than=2048 -fno-stack-protector -fomit-frame-pointer -fno-var-tracking-assignments -g -Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=implicit-int -Werror=strict-prototypes -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -mconstant-gp -DKBUILD_BASENAME='"shmem"' -DKBUILD_MODNAME='"shmem"' -c -o mm/.tmp_shmem.o mm/shmem.c
[Bug middle-end/83653] [6 Regression] GCC fails to remove a can't-happen call on ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83653 --- Comment #2 from Matthew Wilcox --- 7.2, 6.2, 5.5 and 4.9 fail. 4.6.3 succeeds.
[Bug middle-end/83653] [6/7/8 Regression] GCC fails to remove a can't-happen call on ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83653
--- Comment #5 from Matthew Wilcox ---
Hi Aldy!
Thanks for looking into this. Yes, I agree, there's no way that GCC can know
this is a constant, but that *should* have been taken care of. Please pardon
me copying and pasting from the original source file rather than the
preprocessed source, but I find it utterly impossible to work with the
preprocessed source ...
#define atomic_sub_return(i,v) \
({ \
int __ia64_asr_i = (i); \
(__builtin_constant_p(i)\
&& ( (__ia64_asr_i == 1) || (__ia64_asr_i == 4) \
|| (__ia64_asr_i == 8) || (__ia64_asr_i == 16) \
|| (__ia64_asr_i == -1) || (__ia64_asr_i == -4) \
|| (__ia64_asr_i == -8) || (__ia64_asr_i == -16)))\
? ia64_fetch_and_add(-__ia64_asr_i, &(v)->counter) \
: ia64_atomic_sub(__ia64_asr_i, v); \
})
That __builtin_constant_p() *should* have led GCC to throw up its hands, not
bother checking for the +/- 1, 4, 8, 16 cases and just call ia64_atomic_sub().
Looking at the disassembly, I see a BBB bundle, indicating quite strongly to me
that it is testing for all of these cases, and the __builtin_constant_p is
being ... ignored? misunderstood?
Thanks!
[Bug middle-end/83653] [6/7/8 Regression] GCC fails to remove a can't-happen call on ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83653 --- Comment #7 from Matthew Wilcox --- OK, so how should we write this function/macro to accomplish what we want? And the requirement is "If the argument is one of these eight special constants, use this special instruction, otherwise call this function". Even if the argument happens to be one of the eight special values at runtime, we should still call the function, because it's not worth doing runtime checks; we only want a compile-time optimisation. And it's worth the compile-time optimisation because adding constant 1/-1 is really, really common.
[Bug middle-end/83653] [6/7/8 Regression] GCC fails to remove a can't-happen call on ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83653 --- Comment #9 from Matthew Wilcox --- Maybe I'm a little slow, but I don't see what the path is that sets 'nr' to 0. It's 1UL << compound_order. Typically, compound_order is 0, although it may be anything up to log2(number of pages in the machine). Are you saying that nr could be 0 because DOM2 thinks compound_order() could be larger than 64, and thus undefined?
[Bug middle-end/83653] [6/7/8 Regression] GCC fails to remove a can't-happen call on ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83653 --- Comment #11 from Matthew Wilcox --- I'm sorry, I still don't get it. What I think you're saying is that GCC performs this optimisation: nr = 1UL << compound_order(page); atomic_sub_return(x, nr); into: if (PageHead(page)) atomic_sub_return(x, 1); else atomic_sub_return(x, 1UL << page[1].order) That seems like a great optimisation to me, and I'm all for it. What I don't understand is where the b_c_p call gets lost. Also, this bug is pretty fragile. If I replace the 'nr' in the call to atomic_sub_return with '1UL << compound_order(page)', the bug goes away. Anyway, I have no vested interest in ia64 or the code that's currently using __b_c_p. I just want it to stop blocking me getting my patch in. It's a bit different from 72785 because I can't just resolve it by removing the checking code. Just tell me what the macro should look like.
[Bug middle-end/83653] [6/7/8 Regression] GCC fails to remove a can't-happen call on ia64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83653 --- Comment #14 from Matthew Wilcox --- Confirmed this fixes the problem. I'll send it to Tony and see if he likes it. May I add your Signed-off-by to the patch?
[Bug c/85053] New: free not listed as built-in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85053
Bug ID: 85053
Summary: free not listed as built-in
Product: gcc
Version: 8.0.1
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: matthew at wil dot cx
Target Milestone: ---
The function free() is not listed as a built-in in the manual, but the source
code has several references to BUILT_IN_FREE. I believe the manual should be
adjusted as so:
-@code{fprintf}, @code{fputs}, @code{frexp}, @code{fscanf},
+@code{fprintf}, @code{fputs}, @code{free}, @code{frexp}, @code{fscanf},
(not submitting this as a real patch because I don't have a copyright
assignment on file. this patch shouldn't need one!)
[Bug c/85055] New: warn on accessing free memory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85055
Bug ID: 85055
Summary: warn on accessing free memory
Product: gcc
Version: 7.3.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: matthew at wil dot cx
Target Milestone: ---
GCC knows that a pointer passed to free() is no longer valid, but it misses an
opportunity to warn here:
char f(char *foo)
{
free(foo);
return foo[0];
}
[Bug c/99997] Missed optimisation with -Os
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=7 Matthew Wilcox changed: What|Removed |Added CC||matthew at wil dot cx --- Comment #1 from Matthew Wilcox --- Actually, it should eliminate the one _after_ the L2 label. It just moved the constant 1 into %eax and doesn't need to limit it again.
[Bug c/97287] New: Warn for expanding range of an arithmetic type
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97287
Bug ID: 97287
Summary: Warn for expanding range of an arithmetic type
Product: gcc
Version: 11.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: matthew at wil dot cx
Target Milestone: ---
I've just fixed multiple instances of bugs that look a lot like function f()
when they should have been function g(). This affects filesystems in Linux
which have to remember to cast an unsigned long to an off_t before shifting (or
there will be a bug on 32-bit kernels when dealing with files that are larger
than 4GB).
When I looked for a warning option to add, I thought -Warith-conversions might
do the job, but it doesn't. Maybe this functionality should be added there, or
maybe it should have its own warning.
I think what we're looking for is an operation which expands the range of the
type (left shift, multiplication, addition; maybe subtraction?) and the rules
of C require the operation to be done in a narrower type, but the result is
assigned to a wider type.
unsigned long long f(unsigned int i) { return i * 4096; }
unsigned long long g(unsigned int i) { return i * 4096ULL; }
[Bug c/101645] New: -Wsign-conversion misses negation of unsigned int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101645
Bug ID: 101645
Summary: -Wsign-conversion misses negation of unsigned int
Product: gcc
Version: 12.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: matthew at wil dot cx
Target Milestone: ---
Test case:
unsigned long a(void);
void b(long);
void c(void) {
unsigned int x = a();
b(-x);
}
There is a missed warning in the call to b(). x is negated, but as an int. It
is then passed to b() which sees a very large positive number instead of a
small negative number. This has recently affected the Linux codebase, and it's
disappointing that gcc doesn't have a warning that we could enable to find it.
https://godbolt.org/z/xvaxh1rqr
shows that Clang does spot this with -Wsign-conversion, but GCC currently
doesn't (it only warns for line 4 and not line 5). Admittedly the Clang
diagnostic message isn't the greatest, but at least it says _something_.
[Bug c/101645] warn about neg of unsigned type should be added to -Wsign-conversion
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101645
--- Comment #4 from Matthew Wilcox ---
On second thoughts -Wsign-conversion is useless. Consider that it warns on
this:
unsigned long f(unsigned long x, int y) {
return x + y;
}
Both gcc and clang emit a warning, which makes it useless. That code obviously
does what the programmer intended.
What would be useful is a warning which diagnoses code which behaves
differently in infinite-precision arithmetic vs the C promotion rules.
For example,
unsigned long g(unsigned char v, int s) {
return v << s;
}
By the C standard, v is promoted to int, shifted by s and then promoted to
unsigned long. This is a fertile source of bugs as it's easy to overlook that
v will not be promoted to unsigned long first.
(by the way, this is another example where clang diagnoses with -Wsign-compare
and gcc doesn't.)
Another example plucked from real life:
long long h(unsigned int x) {
return x * 4096;
}
(slightly changed to demonstrate the problem on LP64; the original was from
unsigned long to long long, and it is only a bug on ILP32)
