search

intel / llvm

Intel staging area for llvm.org contribution. Home for Intel LLVM-based projects.
Other
1.21k stars 727 forks source link

Base64::encode/decode is not standard base64 enconding #14729

Open jzc opened 1 month ago

jzc commented 1 month ago

Describe the bug

The Base64::encode/decode methods, which are used to encode binary data in the .prop files used by sycl-post-link, is not the standard base64 encoding (RFC 4648): Base64::encode("hello world", ...) gives oVGbs9GI39mcsRG while standard base64 encodes this as aGVsbG8gd29ybGQ=. The pair of methods do not seem to be flawed in the case of a being able to encode/decode successfully, but this makes it awkward and confusing to make new sycl-post-link tests checking the output of the .prop files, as they use a nonstandard encoding.

To reproduce

No response

Environment

No response

Additional context

No response

bader commented 1 month ago

I don't think this issue should be classified as a bug. This implementation detail has no impact on product functionality.

AlexeySachkov commented 1 month ago

Ideally, we shouldn't write sycl-post-link tests which have Base64-encoded strings in CHECK lines, because they are hard to verify and review. There should be a different mechanism for testing, but coming up with one has always been a lower priority, even though @maarquitos14 had a patch to use llvm debug output mechanism for spec constants pass testing to improve tests readability.

Once we move to clang-offload-wrapper, I think properties tests could be re-written into unit-tests or something like that which should improve the situation.