Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC "stringop-overread" warnings #1079

Closed
niekbouman opened this issue Mar 1, 2022 · 18 comments
Closed

GCC "stringop-overread" warnings #1079

niekbouman opened this issue Mar 1, 2022 · 18 comments
Milestone

Comments

@niekbouman
Copy link

Dear flint-devs,

Maybe this is already known, but when compiling with GCC v11.2.1 I get several "stringop-overread" warnings, like:

In file included from set_fmpz_poly.c:12:
In function ‘fq_default_poly_set_fmpz_mod_poly’,
    inlined from ‘fq_default_poly_set_fmpz_poly’ at set_fmpz_poly.c:33:4:
/flint/fq_default_poly.h:510:7: warning: ‘fq_zech_poly_set_fmpz_mod_poly’ reading 88 bytes from a region of size 8 [-Wstringop-overread]
  510 |       fq_zech_poly_set_fmpz_mod_poly(rop->fq_zech, op, ctx->ctx.fq_zech);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

kind regards,
Niek

@wbhart
Copy link
Collaborator

wbhart commented Mar 2, 2022

Thanks for the report. I don't have any insight but I'll mark it as needing to be fixed for the next release.

@wbhart wbhart added this to the flint-2.9 milestone Mar 2, 2022
@albinahlback
Copy link
Collaborator

I think I looked at this 6 months ago, and I think it was because of ctx structures. Don't know why. You get alot more of these in Arb when doing stuff like arb_midref(x) where x is const arb_t and such.

@albinahlback
Copy link
Collaborator

Please see flintlib/arb#407 for my suggestion.

@wbhart
Copy link
Collaborator

wbhart commented Apr 25, 2022

@albinahlback I have no idea what this issue is about. We are planning to do a 2.9 release by the end of next week or so. Would it be possible for you to investigate in the meantime and make a recommendation or better yet a PR to address this?

stringop-overread sounds like it should be to do with strings, so I don't completely understand how this could be related to our structs. Perhaps an explanation on this ticket could also help clear that up.

When I try to Google stringop-overread I get a whole pile of links to issues about "bogus stringop-overread" messages and people complaining they have encountered this issue, but not information on what it actually is.

@albinahlback
Copy link
Collaborator

albinahlback commented Apr 26, 2022

Yeah, GCC 11 complains when you have some function on the form

int
foo(const mystruct input[FIXED_NUMBER]);

and only inputing a pointer (as compared to an array of said size) via

mystruct * ptr = /* whatever */;
myfunction(ptr);

I was thinking of waiting until GCC 12 was being released, as I read somewhere that something similar to this should have been fixed in the master branch. However, as it still hasn't been released yet, I am going to compile the current head of GCC now to see if the problem is still there.

@albinahlback
Copy link
Collaborator

It is still relevant:
2022-04-26_17

@albinahlback
Copy link
Collaborator

Perhaps it is also relevant to ping @fredrik-johansson here. As mentioned in another issue, I think a change to XXX_[src]ptr would be most suiting. I suspect this can be one reason why GMP and MPFR follows such a naming scheme.

@fredrik-johansson
Copy link
Collaborator

I'm puzzled by the fact that unity_zp_coeff_inc generates the warning your screenshot but unity_zp_coeff_add doesn't. They contain the same kind of context object access.

@albinahlback
Copy link
Collaborator

Yeah, I have no idea actually. Perhaps it the generated code gets generated in such a way that that it tries to fetch that data, while it doesn't do that for other functions. I don't know.

@fredrik-johansson
Copy link
Collaborator

What is a minimal code example that triggers this warning? gcc11 compiles the following without problems for me:

typedef int int_ref[1];

int f(int_ref x)
{
    return x[0];
}

int main()
{
    int x = 0;
    int * y = &x;
    return f(y);
}

@albinahlback
Copy link
Collaborator

Yeah, I've tried similar things with no result.

Do you get these warnings when compiling FLINT (or Arb)? Is it GCC 11.2 you're using?

@fredrik-johansson
Copy link
Collaborator

No. I installed it from ubuntu-toolchain-r which only provides version 11.1.

@albinahlback
Copy link
Collaborator

Obtained GCC 12 today on Arch Linux. This message still pops up.

@wbhart
Copy link
Collaborator

wbhart commented May 12, 2022

So I think we need to find a minimal example and then report it. This is indeed a difficult issue.

@wbhart
Copy link
Collaborator

wbhart commented May 16, 2022

As this is a warning only and @tthsqe12 and I feel the code is 100% valid (so that the warning is invalid), I suggest we hold over this ticket until the next release.

@fredrik-johansson
Copy link
Collaborator

Should we disable the warning in our CFLAGS to prevent noise?

@wbhart
Copy link
Collaborator

wbhart commented May 16, 2022

I was thinking of just mentioning it in our release. It only affects GCC 11 and 12 as far as we can tell.

If we put something in the CFLAGS, chances are it breaks something else, or we forget about it and it causes more problems than it solves.

I think some of the distributions probably build with warnings set to cause errors anyway, so I don't think there is much we can do about this. Even if we could be absolutely certain this and the other ticket were GCC bugs it is likely years before they fix it (based on the other similar bugs reported).

I'm just reluctant to hide something like that from users and also very reluctant to rewrite our code for what seems like a bug that we had nothing to do with. In theory we could set a pointer to NULL in one case to suppress it. For this PR I don't know a workaround at all.

@albinahlback
Copy link
Collaborator

I believe this is an issue that is considered a bug in GCC 12, so for the moment I am going to close it. Feel free to reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants