mit-pdos / xv6-riscv

Xv6 for RISC-V
Other
6.58k stars 2.38k forks source link

Does `user/sh.c` violate the strict aliasing rules? #196

Open yuxqiu opened 10 months ago

yuxqiu commented 10 months ago

The C standard Section 6.5 defines the strict aliasing rule as follows.

An object shall have its stored value accessed only by an lvalue expression that has one of the following types:

  1. a type compatible with the effective type of the object,
  2. a qualified version of a type compatible with the effective type of the object,
  3. a type that is the signed or unsigned type corresponding to the effective type of the object,
  4. a type that is the signed or unsigned type corresponding to a qualified version of the effective type of the object,
  5. an aggregate or union type that includes one of the aforementioned types among its members (including, recursively, a member of a subaggregate or contained union), or
  6. a character type.

In user/sh.c, all pointers to different types of commands are converted to struct cmd*:

https://github.com/mit-pdos/xv6-riscv/blob/f5b93ef12f7159f74f80f94729ee4faabe42c360/user/sh.c#L196-L221

The returned pointer is then used to access the type field:

https://github.com/mit-pdos/xv6-riscv/blob/f5b93ef12f7159f74f80f94729ee4faabe42c360/user/sh.c#L71-L130

At this point, it seems to me that this access (cmd->type) violates the strict aliasing rules, since struct cmd* and struct execcmd* (for example) cannot be aliased:

  1. struct cmd is not compatible with struct execcmd.
  2. Same as above.
  3. struct cmd is not a signed/unsigned type.
  4. Same as above.
  5. struct execcmd does not contain any members whose types are compatible with struct cmd.
  6. struct cmd is not a character type.

IMHO, the correct thing to do is to replace int type; with struct cmd type;. By doing so, we can then alias struct cmd* and struct execcmd*. CPython does the same (PEP 3123).


(Although this project is written in ANSI C, the C version is not specified as a compiler flag. Therefore, I assume it should follow the C17 standard as the version of gcc installed on my machine is 12.2.0, which uses -std=gnu17.)