This is an archive of the discontinued LLVM Phabricator instance.

[AVR][clang] Fix a bug in AVR toolchain search paths
ClosedPublic

Authored by benshi001 on Jan 27 2021, 7:39 AM.

Details

Summary

Current AVR clang driver assumes sysroot to be fixed "/" and omits
user specified "--sysroot" option. And this patch fixes that.

Diff Detail

Event Timeline

benshi001 created this revision.Jan 27 2021, 7:39 AM
benshi001 requested review of this revision.Jan 27 2021, 7:39 AM
MaskRay added inline comments.Jan 27 2021, 11:38 AM
clang/lib/Driver/ToolChains/AVR.cpp
374

This should call ToolChain::GetProgramPath instead

benshi001 updated this revision to Diff 319812.Jan 28 2021, 3:03 AM
benshi001 marked an inline comment as done.EditedJan 28 2021, 3:11 AM

There is a use of GetProgramPath at above line.

std::string Linker = getToolChain().GetProgramPath(getShortName());

I use a better solution that add user specified sysroot to ProgramPath via GetProgramPaths.push_back().

The test "make check-clang-driver -j8" passes both with and without avr-ld/avr-libc installed.

MaskRay added inline comments.Jan 28 2021, 11:32 AM
clang/lib/Driver/ToolChains/AVR.cpp
431

My feeling is that empty --sysroot should mean /, instead of /usr. You probably need to read other ToolChains/*.cpp files first

benshi001 updated this revision to Diff 320015.Jan 28 2021, 6:16 PM
benshi001 marked an inline comment as done.
benshi001 added a comment.EditedJan 28 2021, 6:20 PM

I also think that sysroot means /, but the basic_riscv32_tree seems to assume sysroot to be /usr. However I change my patch to / as you suggested.

benshi001 retitled this revision from [AVR][clang] Fix a bug in AVR clang driver to [AVR][clang] Fix a bug in AVR toolchain search paths.Jan 28 2021, 6:46 PM
MaskRay added inline comments.Jan 28 2021, 8:05 PM
clang/lib/Driver/ToolChains/AVR.cpp
339

Would be nice not needing to hard code /usr/bin. How do other toolchains do?

342

Mixed tab and space?

429

= getDriver().SysRoot

benshi001 marked 3 inline comments as done.Jan 28 2021, 11:24 PM
benshi001 added inline comments.
clang/lib/Driver/ToolChains/AVR.cpp
339

Would be nice not needing to hard code /usr/bin. How do other toolchains do?

Many toolchains just use GCC's program paths, which auto include user specified sysroot. So I simplify it use AVR-GCCInstallation as first choice, then class ToolChain's default behavior.

benshi001 marked an inline comment as done.Jan 28 2021, 11:24 PM
dylanmckay accepted this revision.Feb 1 2021, 10:05 PM
This revision is now accepted and ready to land.Feb 1 2021, 10:05 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 2 2021, 6:46 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript