Page MenuHomePhabricator

Fix PR44093: use of stale error stream when linking
AbandonedPublic

Authored by thopre on Wed, Nov 20, 1:11 PM.

Details

Summary

llvm::lld::link() calls llvm::lld::enableColors() before initializing
the streams llvm::lld::stdoutOS and llvm::lld::stderrOS. This is a
problem because enableColors() uses stderrOS indirectly via a call to
lld::errs(). It can lead to random behavior if the lifetime of the
object passed as stderrOS in link() expires before a subsequent
invocation of link().

This commit remove enableColors() altogether and replace it by calls to
enable_colors(), making it explicit on which stream to enable colors.
link() is then changed to invoke enable_colors() on stderrOS.

Event Timeline

thopre created this revision.Wed, Nov 20, 1:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Nov 20, 1:11 PM
thopre edited projects, added lld; removed Restricted Project.Wed, Nov 20, 1:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Nov 20, 1:45 PM
ruiu added a subscriber: ruiu.Wed, Nov 20, 3:58 PM
ruiu added inline comments.
lld/COFF/Driver.cpp
76

Let's remove enableColors and replace it with stderrOS..enable_colors(stderrOS.has_colors()). I wrote that single-line function but in hindsight it doesn't provide much value.

thopre updated this revision to Diff 230413.Thu, Nov 21, 3:03 AM
thopre marked an inline comment as done.

Remove enableColors and replace calls to it by the underlying enable_colors()

thopre edited the summary of this revision. (Show Details)Thu, Nov 21, 3:04 AM
ruiu added a comment.Thu, Nov 21, 8:33 PM

James Knight has submitted a very similar change to fix the problem.

thopre abandoned this revision.Fri, Nov 22, 2:07 AM

James Knight has submitted a very similar change to fix the problem.

Indeed, awesome.