This is an archive of the discontinued LLVM Phabricator instance.

[dfsan] Add origin ABI wrappers
ClosedPublic

Authored by stephan.yichao.zhao on Mar 15 2021, 9:24 AM.

Details

Summary

supported: bcmp, fstat, memcmp, stat, strcasecmp, strchr, strcmp,
strncasecmp, strncp, strpbrk

This is a part of https://reviews.llvm.org/D95835.

Diff Detail

Event Timeline

stephan.yichao.zhao requested review of this revision.Mar 15 2021, 9:24 AM
stephan.yichao.zhao created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2021, 9:24 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
morehouse added inline comments.Mar 15 2021, 2:47 PM
compiler-rt/lib/dfsan/dfsan_custom.cpp
119
123–129
143

This function is always called after dfsan_strchr. Should we just combine them?

164
216
228
234

Is the casting necessary?

237
242–244
246–249
317
326

These lines are unreachable.

stephan.yichao.zhao marked 9 inline comments as done.

updated

compiler-rt/lib/dfsan/dfsan_custom.cpp
164

Thank you.

228

This function is called by other wrappers too: bcmp, strcmp, strncmp, ... so we give it a general name.
I fixed the typo prefixs to prefixes.

237

This is similar to dfsan_get_label_of_string_prefixs. It is used by multiple wrappers.
Fixed typo.

317

done. replaced all pos -> bytes_read

326

removed. and fixed other similar cases.

morehouse added inline comments.Mar 16 2021, 10:41 AM
compiler-rt/lib/dfsan/dfsan_custom.cpp
228

Yes, but ultimately it is getting the label from comparing two byte sequences (i.e. memcmp). I was just suggesting a shorter name, but don't feel too strongly about it.

318–319
365–366
398

For simplicity, can we combine this with dfsan_strcmp? For example we could make n a ssize_t and do a normal strcmp when it's -1.

461

Can we also combine this with dfsan_strcasecmp?

compiler-rt/test/dfsan/custom.cpp
1646

Can we move test_fork up to keep the lexicographic ordering?

stephan.yichao.zhao marked 6 inline comments as done.Mar 16 2021, 11:24 AM
stephan.yichao.zhao added inline comments.
compiler-rt/lib/dfsan/dfsan_custom.cpp
228

I misunderstood that. renamed to dfsan_get_memcmp_label and dfsan_get_memcmp_origin.

398

Using ssize_t reduces the size of n this function can accept.

__dfsw_strncmp always ensures n > 0 before calling dfsan_strncmp. so we could use n==0 as a flag to distinguish the two cases.

stephan.yichao.zhao marked 2 inline comments as done.

refactored code

This revision is now accepted and ready to land.Mar 16 2021, 4:02 PM
This revision was landed with ongoing or failed builds.Mar 16 2021, 7:23 PM
This revision was automatically updated to reflect the committed changes.