- User Since
- May 16 2019, 3:50 PM (75 w, 12 h)
Aug 12 2020
Aug 11 2020
Aug 6 2020
Aug 5 2020
Missing definition in spec/stdc.td
Aug 4 2020
FWIW, I think in most real world scenarios the O(N*M) approach would be faster. Granted I don't know what those scenarios are I had never heard of this function, I have no idea where this is useful.
Jul 31 2020
Clearly I'm too late but there are some problems with this revision.
Jul 30 2020
Jul 29 2020
This looks good. I wonder though how we will deal with global variables. Those can't be easily wrapped like functions can.
Jul 24 2020
Jul 21 2020
Jul 20 2020
Jul 17 2020
Jul 16 2020
Why add support to the TEST macros for this when we have the MPFR macros which I think handle floats better than we can with TEST. Comparisons with floating points are not fun, its why we use the mpfr library in the first place. Is there even a need to use the TEST macros with floats?
Jun 26 2020
Would you mind looking if it is possible for a program not to be linked against LibSystem.dylib which contains Apple's libc? I thought it was the case that it wasn't possible, but I might be wrong.
Jun 23 2020
Jun 18 2020
FWIW, this is a good candidate for fuzz testing
Are you using the arc tool to create these? I've always had trouble with it too. You can create a diff and upload that diff or copy and paste it in Phab, which I think is a lot easier and less error prone. git diff origin -U99999 > patch then you can upload patch from reviews.llvm.org. Hopefully thats helpful
Jun 9 2020
This makes sense to me. We should look to do this later then. It makes sense to move on now.
I don't know the solution here, but there is so much which we end up copying between configs of architectures. It's most apparent with the math.h targets because our implementations are architecture and OS agnostic, but most of libc except for syscall like you mention is too. I think it's clear that having separate build config files for every OS * architecture is untenably difficult for us to maintain. I should have mentioned this earlier, it's not this patch which introduced this, but I didn't catch it then.
May 30 2020
Oh yes you two are right last I looked must have been before that change. Sorry for the noise.
May 28 2020
FWIW, we should do something about clang and clang-tools-extra being required. It also causes the Harbormaster builds to fail because it not longer builds all top level targets anymore.
May 21 2020
May 16 2020
This looks good to me but it might be worth waiting for @MaskRay if he has anything to say because I missed this the first time.
May 15 2020
May 13 2020
May 7 2020
May 6 2020
May 5 2020
May 4 2020
May 3 2020
I don't want to make any suggestions yet because I don't fully understand why we need a MPFR_TEST. Would you mind clarifying the need for TEST_WITH_BASE_CLASS? Instead of MPFR_TEST could we have TEST_EXTENSION's first argument take a class with compare, would that simplify it? I'm by no means a gtest expert, I'm not sure how someone would use gtest to fill these kinds of needs. Maybe they would make their own macros which end up calling FAIL or using matchers. We aren't limited to that though so perhaps these would end up being more complicated than this solution.
LG to me too
May 1 2020
Apr 30 2020
Apr 29 2020
Apr 28 2020
Apr 26 2020
If the goal is to not need to explicitly use the __llvm_libc:: prefix in our tests we could change the TEST macro to put the class it makes in __llvm_libc instead of needing to remember to explicitly put our tests in __llvm_libc. Granted it isn't difficult to remember to put new tests in the namespace but I don't think there is any clear advantage to requiring it either.
Apr 25 2020
Apr 24 2020
Apr 23 2020
Oops, thanks for fixing this
Apr 22 2020
I agree, sanitizers are useful for detecting problems in programs using libc and detecting problems with the libc itself. It's really the "fast path" where we want the second kind, because it's more complicated and harder to see if it's safe. @sivachandra is right, and thats not an example I would want to set here.
I'm not sure if this discussion belongs here or D77949, but as this complicates that patch even more I will do so here.
Do these all belong in a linux specific directory? Wouldn't the two which aren't x86 specific be fine to put into libc/src/__support/?
Apr 21 2020
Apr 20 2020
Excited about this one!
Apr 17 2020
Move "hello" to a variable
Add ssize_t to __posix-types.h
Apr 15 2020
Apr 14 2020
Remove fwrite_unlocked from internal header
Internally refer to FILE by its fully qualified name __llvm_libc::FILE NFC
Apr 13 2020
Remove cookie from the base FILE type
- Use incomplete struct FILE as FILE's definition not void
- Remove cookie from write_function_t
Apr 12 2020
- Change FILE's external definition from empty struct to void
- Remove change to syscall_test.cpp
Apr 11 2020
Gentle ping that this patch is still broken when building with UBSan
As a high level comment, this scheme is technically safe because no mmu that I know of can deal make fault pages on non word boundaries (of course they all do at page boundaries) so readying some extra bytes past the end of the string will never cause SIGBUS or SIGSEGV. However it is still undefined behavior and sanitizers will complain. If you build with LLVM_USE_SANITIZER=Address I'm pretty sure that FourBytes and TwoBytes will fail. And certainly __llvm_libc::strlen(__llvm_libc::strcpy(::malloc(6), "12345")); will get the sanitizers to complain.
Apr 10 2020
Apr 6 2020
Just some nits inline.
One thing though, this change is now wider than the original description. So, update the description here (and also in your local git repo if you use git llvm push.)
It might also be useful to add [NFC] to the name while you're at it.
Also, although we only want header files to have the C++ tag, shouldn't we also left justify the text in our .cpp files too?
Apr 3 2020
LGTM. I think the shortened name with the . prefix is a little confusing personally, but I can also see how it is useful.