1364234 - Standard library objects inconsistently contain alignas() members complained about by the MOZ_NON_PARAM analysis

Status: NEW

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Nika Layzell [:nika] (ni? for response)]

The MOZ_NON_PARAM analysis now complains about types which contain alignas(_) marked members as of bug 1339537. Unfortunately, standard libraries occasionally change their implementations, and are often adding alignas(_) members to their structs, which suddenly our analysis rejects when passed by value as a parameter.

For example, when Christian was trying to run the static analysis locally, he ran into the problem that in his standard library, std::list contained an alignas(_) member. When we were writing the static analysis, the problem that on OS X std::function contained an alignas member also came up.

I don't know how real the problem of ABI not being defined properly for alignas(_) members is, but it is definitely inconvenient to have the analysis not run on arbitrary people's computers due to differences in their implementation of the standard library.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

What do you think we can/should do here?

Comment 2

[(no longer active)]

Can you remind me what actual problem we are solving here again?  :-)

Comment 3

[Nika Layzell [:nika] (ni? for response)]

(In reply to :Ehsan Akhgari (super long backlog, slow to respond) from comment #2)
> Can you remind me what actual problem we are solving here again?  :-)

The problem is that, according to the C++ standard, alignas(_) has no effect on ABI. This means that in ABIs where structs are passed on the stack (such as x86), alignment is not guaranteed for arguments to functions which are passed by value. This is according to a warning produced by MSVC (see bug 1287006 comment 26).

This shouldn't affect platforms which don't have an ABI affected by this quirk of the spec, but might have an impact on win32 (where the value is already an error due to -Werror), or on other 32-bit platforms.

Unfortunately for us, many standard library types arbitrarially use alignas(_) inside of them, and with this analysis enabled, they can no longer be passed by parameter.

Comment 4

[Nathan Froyd [:froydnj]]

1. Add things to the whitelist.
2. Report issues to the standard library maintainers, preferably with testcases demonstrating the issues, so they stop doing this.

Comment 5

[Nika Layzell [:nika] (ni? for response)]

(In reply to Nathan Froyd [:froydnj] from comment #4)
> 1. Add things to the whitelist.
> 2. Report issues to the standard library maintainers, preferably with
> testcases demonstrating the issues, so they stop doing this.

Currently the whitelist only tests namespaces for consumers, rather than the types themselves. For example, I allow functions in std:: to accept an alignas(_) type by value, but don't allow non-std:: functions to accept a std:: alignas(_) type by value.

Are you suggesting changing this around and whitelisting these std:: types for passing-by-value anywhere Nathan?

Comment 6

[(no longer active)]

If I understand this correctly, we have the following facts on our hands:

a) The ABI issues are x86 only.
b) The platform we are hitting this on (OSX) doesn't support x86 any more.

Is this true?  If both of the above are true, it seems to me that we should ignore these and file bugs on libc++ to get them to fix their library.

Comment 7

[Nathan Froyd [:froydnj]]

(In reply to :Ehsan Akhgari (super long backlog, slow to respond) from comment #6)
> If I understand this correctly, we have the following facts on our hands:
> 
> a) The ABI issues are x86 only.
> b) The platform we are hitting this on (OSX) doesn't support x86 any more.
> 
> Is this true?  If both of the above are true, it seems to me that we should
> ignore these and file bugs on libc++ to get them to fix their library.

I am about 80% certain that the same issues are present on ARM and AArch64, which we'd have to care about for Fennec (where we also use libc++).  But we also have to care about x86 on Fennec as well, where these issues would be present, too; we just don't notice them because we're not running static analysis there.