Page MenuHomePhabricator

[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
126–129
174

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

195
207
214–215
215
217–220
219
225

Is the casting necessary?

301
310

These lines are unreachable.

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

updated

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

Thank you.

215

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

219

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.

301

done. replaced all pos -> bytes_read

310

removed. and fixed other similar cases.

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

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.

302–303
348–349
369

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.

452

Can we also combine this with dfsan_strcasecmp?

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

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
219

I misunderstood that. renamed to dfsan_get_memcmp_label and dfsan_get_memcmp_origin.

369

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.