[libc-commits] [PATCH] D158774: [libc] Add GPU support for `printf` and `fprintf`

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/src/stdio/gpu/parser.h:134
+    ++cur_pos;
+
+  if (format[cur_pos] != '\0')
----------------
jhuber6 wrote:
> michaelrj wrote:
> > I realize that this was probably for size reasons, but this should probably add back something for parsing flags, widths, and precisions. The current design will accept all valid format specifiers, but won't reject all invalid format specifiers (e.g. `%?d`).
> I originally had code for that but it added an absurd amount of resource usage that I don't think we really need. The parsing here will definitely pick this up as an argument and copy the argument to the server. We use a real parser there so the actual printing will pick it up, but I guess the number of arguments might be off?
> 
> I'm guessing the standard solution is to just ignore it?
in the case of an invalid format specifier (like `%?d`) the behavior is undefined. The way we generally handle it is by treating the format specifier (in this case `%?`) as a raw string. The current micro-parser would mistakenly skip past the `?` and assume the format name is `d`, which is just a letter `d` in this case. This would cause argument misalignment since there would be an extra `int` argument where there shouldn't be one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158774/new/

https://reviews.llvm.org/D158774