Closed balazske closed 6 years ago
There is a following definition in select.h
:
/* fd_set for select and pselect. */
typedef struct
{
/* XPG4.2 requires this member name. Otherwise avoid the name
from the global namespace. */
#ifdef __USE_XOPEN
__fd_mask fds_bits[__FD_SETSIZE / __NFDBITS];
# define __FDS_BITS(set) ((set)->fds_bits)
#else
__fd_mask __fds_bits[__FD_SETSIZE / __NFDBITS];
# define __FDS_BITS(set) ((set)->__fds_bits)
#endif
} fd_set;
The already imported struct is the one with __fds_bits
, the to be imported is one with fds_bits
. These are structurally not equivalent (one field with different name). What to do in this case?
If the conflicting case would result in failed import of the record, a !hasBody
assertion occurs somewhere (not checked if it is related but possibly yes). A possibility is to merge the definitions, import the new fields into the existing struct (but it can be that there are fields with different type but same name, these can not be imported, and a AST structure is created that does not correspond to source code).
Probably the compile commands or include files are wrong?
Yes, this is indeed a hard situation.
In this case I would create another fd_set
with the fds_bits
.
At the moment, we do not create the second fd_set
, right?
Having one struct with both fields is probably not correct.
Still, we will have a very hard time to distinguish this situation from that one when the structs are indeed inequivalent (ODR is violated). Perhaps, we should track the macro definitions on which the definition of a record is dependent upon?
Adding @dkurpp and @zporky, maybe they have some better ideas.
My other thought: Strictly speaking this is ODR violation! So we could diagnose the ODR violation and then exit (which is not equal to a crash).
Adding @dkrupp
Here are my 50 cents: This is ODR violation, and we cannot handle this in the ASTImporter level, clang should exit in this case.
The error must be handled on the build system / compile commands json level. Two TUs, one with __USE_XOPEN
and another without it could never have been linked into one lib/executable (that is undefined behaviour). Thus, we may not expect to analyse together such two TUs.
Adding @whisperity , to hear other opinions too.
I think it is not good to have multiple records (struct or other) with same name (and different content). The current implementation does this, but something goes wrong with import of the field and the crash happens. (The two different struct's with same name are linked together with previous decl chain, this is wrong to do. If there is somewhere a map-like structure for name lookup it can not work with multiple things with same name.)
Probably the handleNameConflict
should be implemented to generate a new name?
Related pull request: