Make sparse work from travis #100
Conversation
The sparse introduction patch did not get enough testing, this broke native stdatomic detection on all systems. Fixes: eaceda6 ("buildlib/fixup-include/endian.h: Introduce this header file") Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
The constants are in host order. INADDR_ANY is 0 so the swap has no change, but the INADDR_LOOPBACK test in rsocket never worked. Found by sparse Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
This is motiviated by sparse not handling the transparent union in wait() properly, but using waitpid is generally a better idea since we know what is supposed to be waited for here. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
No sense in having a private API here. stdatomic will be equivalent to the gcc builtin. This is motivated by sparse which does not handle the gcc atomic builtins. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
These swaps were missed or in the wrong place Found by sparse. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Put it on the prototype and the declaration. Sparse complains if the attribute is not in exactly the same spot, so always put it in the usual place for declarations. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Turns out we need quite a bit more than just a simple wrapper around endian.h. Use a patching scheme instead, we store diffs we want to apply and a script directly modifies the system glibc headers. The shim headers are designed to compile, not to work. When building with sparse the resulting binaries are garbage. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
These are mirrors of the kernel structs that have annotations. Each of the structures is a memory image of BE data, so every > 8 bit quantity needs swapping to access. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
This has one code changes, the sa_addr.lid is stored as BE instead of being converted at use. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
This was done without changing anything, but it seem strange that the pkey stuff returns be values. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
ibv_get_device_guid and ibv_query_pkey are both returning BE values for some reason. imm_data is described to be a BE value, so mark it as such with a transparent union for the alternate to the host endian invalidated_rkey. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
netlink-compat.h redefines IFNAMSIZ unless if.h is included first. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
It is a bit goofy, but gcc requires a unique global name for the implementation when aliasing to the symbol version. So we need to have two prototypes for the same entry point, one with the __ to avoid the warning. This is motivated by sparse which has no way to disable this check. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Sizeof is the more common pattern. Avoids a broken warning in sparse about variably sized stack arrays. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
They are now sparse clean, let us keep things that way. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
|
The changes related to the librdamcm posted to the mailing list look good. 👍 |
| @@ -183,7 +183,7 @@ int umad_status(void *umad); | |||
| ib_mad_addr_t *umad_get_mad_addr(void *umad); | |||
| int umad_set_grh_net(void *umad, void *mad_addr); | |||
| int umad_set_grh(void *umad, void *mad_addr); | |||
| int umad_set_addr_net(void *umad, int dlid, int dqp, int sl, int qkey); | |||
| int umad_set_addr_net(void *umad, __be16 dlid, __be32 dqp, int sl, __be32 qkey); | |||
There was a problem hiding this comment.
Is this going to potentially break apps using this interface?
There was a problem hiding this comment.
There is a slight chance, it changes the underlying types a tiny bit, notably swapping in a short..
However, I am not aware of any architecture that would generate different machine code for this case..
If there is doubt we can symbol version it..
There was a problem hiding this comment.
Ok.
opensm uses this 3 times. All 3 should be fine.
OPA management code does not use this and neither does the infiniband-diags...
So we are probably safe.
|
looks good. |
| -# define htole16(x) (x) | ||
| -# define be16toh(x) __bswap_16 (x) | ||
| -# define le16toh(x) (x) | ||
| +// Simple vesion for sparse with annotations |
There was a problem hiding this comment.
C99 added // comments, we are using C11... But I'll switch it.
|
The goal here is to make sparse run automatically from Travis. We have a lot of control over the Travis environment, so this scheme is unlikely to randomly break inside travis as the glibc will always be the same. sparse in user space turns out to be very hard because sparse and glibc are just not compatible. So we need to change libc headers and no matter what approach is used it will be inherently frail. So.. any solution is going to be compromised in some way or another.. I propose we focus on having it run inside Travis and a few distros that people are interested in. Sparse clearly is finding bugs, so it is more valuable to have something working even if it isn't everything you hoped for. |
|
@jgunthorpe, I'm fine as long as it works :) |
Don't merge this. This is a placeholder to test travis running this branch. I suggest we go ahead after the next release.