This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt][UBSan] Sanitization for alignment assumptions.
ClosedPublic

Authored by lebedev.ri on Nov 15 2018, 10:26 AM.

Diff Detail

Event Timeline

lebedev.ri created this revision.Nov 15 2018, 10:26 AM

Adapt with the actual pointer no longer being provided, compute it by subtracting the offset.
While there, also display the misalignment offest (the result of the real pointer & aligment mask).

Only this compiler-rt part remains.
Would be kind-of cool to have sanitization for this UB even before it is formally added to the C++ library (D54966)

ping @filcab. Also, is there anyone else who is confident reviewing such ubsan handler patches? I do see that it is a very low activity/throughput area of code..

Ping.
Ping @kcc / @rsmith / @filcab / @vsk - do i have the right reviewer list here?

rsmith added inline comments.Dec 20 2018, 4:21 PM
lib/ubsan/ubsan_handlers.cc
133–141

ubsan's diagnostic format is intended to be "like a clang diagnostic", so should be a single sentence. How splitting this into an error pointing at the source location ("assumption [...] failed"), and a note pointing at the memory address that the pointer points to ("address is [...] aligned, misalignment offset is [...] bytes")...

149

... and removing this note.

lebedev.ri marked 2 inline comments as done.

@rsmith thank you for taking a look!
I believe i have addressed the review notes, does this look better?

lebedev.ri added inline comments.Dec 22 2018, 12:29 AM
lib/ubsan/ubsan_handlers.cc
146

Hmm, though one thing extra i forgot to do, is to print "address vs "offset address.
Will adjust later.

Can you test thread_local too?

Can you test thread_local too?

Can you please be more specific? Which test do you want to see?

Can you test thread_local too?

Can you please be more specific? Which test do you want to see?

It has been noted that NetBSD's ld.elf_so does not support alignment and there is a workaround in xray: D56000.

It would be helpful to test similar (harder to debug) scenarios with UBSan and its tests.

Can you test thread_local too?

Can you please be more specific? Which test do you want to see?

It has been noted that NetBSD's ld.elf_so does not support alignment and there is a workaround in xray: D56000.

It would be helpful to test similar (harder to debug) scenarios with UBSan and its tests.

I'm guessing you want me to test that this has some ubsan check, correct? https://godbolt.org/z/5xNnBu
It currently will not, and this check will not do it.

And because @_ZZ18getThreadLocalDatavE10TLDStorage is declared with align 64 attribute,
even if i emit the check, it will be trivially dropped by middle-end.
So i'm not sure how to catch this yet.

Can you test thread_local too?

Can you please be more specific? Which test do you want to see?

It has been noted that NetBSD's ld.elf_so does not support alignment and there is a workaround in xray: D56000.

It would be helpful to test similar (harder to debug) scenarios with UBSan and its tests.

I'm guessing you want me to test that this has some ubsan check, correct? https://godbolt.org/z/5xNnBu
It currently will not, and this check will not do it.

And because @_ZZ18getThreadLocalDatavE10TLDStorage is declared with align 64 attribute,
even if i emit the check, it will be trivially dropped by middle-end.
So i'm not sure how to catch this yet.

I see, pity that it's not expandable right not to TLS allocations.

There is a related workaround in the implementation of TSan for misalignment.

lebedev.ri marked an inline comment as done.

Adjust one message to state that it is talking about offset address.

vitalybuka resigned from this revision.Jan 2 2019, 5:57 PM

Happy 2019 everyone!
Last ping before 8.0 branch.

morehouse accepted this revision.Jan 14 2019, 9:51 AM
morehouse added inline comments.
lib/ubsan/ubsan_handlers.h
32

Nit: Let's move this to right before the matching RECOVERABLE line below.

test/ubsan/TestCases/Pointer/alignment-assumption-attribute-align_value-on-lvalue.cpp
24

Any idea why the assignment doesn't trigger a report while the unused return value does?

This revision is now accepted and ready to land.Jan 14 2019, 9:51 AM
lebedev.ri marked an inline comment as done.Jan 14 2019, 10:32 AM
lebedev.ri added a subscriber: rjmccall.

Oh yay, thank you for the review!

test/ubsan/TestCases/Pointer/alignment-assumption-attribute-align_value-on-lvalue.cpp
24

Because no alignment assumption is being emitted in that case.

The question is, given such a cast that increases the alignment,
if the run-time check is to fail, is the cast is itself UB already,
or is it UB to use the resulting mis-aligned pointer?
(CC @rjmccall, @rsmith)

Regardless, this is mainly a question for a clang side,
and i think it can be addressed as a follow-up.

lebedev.ri marked 2 inline comments as done.

Rebased before commit, addressed @morehouse review note.

lib/ubsan/ubsan_handlers.h
32

Hmm, right.

rjmccall added inline comments.Jan 14 2019, 11:25 AM
test/ubsan/TestCases/Pointer/alignment-assumption-attribute-align_value-on-lvalue.cpp
24

The C standard says the cast is UB; q.v. C99 6.3.2.3p6:

A pointer to an object or incomplete type may be converted to a pointer to a different object or incomplete type. If the resulting pointer is not correctly aligned57) for the pointed-to type, the behavior is undefined.

The C++ standard says the cast has an unspecified result; q.v. C++ N4640 [expr.static.cast]p13:

A prvalue of type “pointer to cv1 void” can be converted to a prvalue of type “pointer to cv2 T”, where T is an object type and cv2 is the same cv-qualification as, or greater cv-qualification than, cv1. If the original pointer value represents the address A of a byte in memory and A does not satisfy the alignment requirement of T, then the resulting pointer value is unspecified.

Actually using that unspecified value would then be UB.

If UBSan enforces alignment at the point of access, that's a significantly weaker check than either standard would allow. The C++ rule is basically the C rule except that the value is allowed to inertly propagate as long as it's not used, which in practice is unlikely — the real difference is that the C rule is extremely easy to enforce and the C++ rule is extremely difficult. All that said, prudentially, I think the rule that UBSan currently enforces is probably the right balance.

lebedev.ri marked an inline comment as done.Jan 14 2019, 11:52 AM
lebedev.ri added inline comments.
test/ubsan/TestCases/Pointer/alignment-assumption-attribute-align_value-on-lvalue.cpp
24

Thank you for chiming in!

I did find that C++ part, and thus didn't really consider this as a bug before.

I suppose we could either emit the check when in C mode;
or be pedantic, and specify that in C++ case a run-time diag should happen,
but indeed, i'm not sure what is best here.
Early detection would be better if you ask me though.

rjmccall added inline comments.Jan 14 2019, 1:31 PM
test/ubsan/TestCases/Pointer/alignment-assumption-attribute-align_value-on-lvalue.cpp
24

Do we have a sanitizer that tries to propagate "poison" values? Something like an uninitialized-variable sanitizer which allows loads of uninitialized variables as long as the loaded value is unused?

thakis added a subscriber: thakis.Feb 5 2019, 8:30 AM

The test seems to flakily fail sometimes:

https://logs.chromium.org/logs/chromium/bb/tryserver.chromium.win/win_upload_clang/478/+/recipes/steps/package_clang/0/stdout

Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90
FAIL: UBSan-Standalone-x86_64 :: TestCases/Pointer/alignment-assumption-attribute-assume_aligned-on-function.cpp (44397 of 46501)
******************** TEST 'UBSan-Standalone-x86_64 :: TestCases/Pointer/alignment-assumption-attribute-assume_aligned-on-function.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';      C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe       -x c   -fsanitize=alignment -O0 C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp -o C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp &&  C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp 2>&1 | FileCheck C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:"
: 'RUN: at line 2';      C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe       -x c   -fsanitize=alignment -O1 C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp -o C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp &&  C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp 2>&1 | FileCheck C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:"
: 'RUN: at line 3';      C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe       -x c   -fsanitize=alignment -O2 C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp -o C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp &&  C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp 2>&1 | FileCheck C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:"
: 'RUN: at line 4';      C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe       -x c   -fsanitize=alignment -O3 C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp -o C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp &&  C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp 2>&1 | FileCheck C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:"
: 'RUN: at line 6';      C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe       -x c++ -fsanitize=alignment -O0 C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp -o C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp &&  C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp 2>&1 | FileCheck C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:"
: 'RUN: at line 7';      C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe       -x c++ -fsanitize=alignment -O1 C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp -o C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp &&  C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp 2>&1 | FileCheck C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:"
: 'RUN: at line 8';      C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe       -x c++ -fsanitize=alignment -O2 C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp -o C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp &&  C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp 2>&1 | FileCheck C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:"
: 'RUN: at line 9';      C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe       -x c++ -fsanitize=alignment -O3 C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp -o C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp &&  C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp 2>&1 | FileCheck C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp --implicit-check-not=" assumption " --implicit-check-not="note:" --implicit-check-not="error:"
--
Exit Code: 1104

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe" "-x" "c" "-fsanitize=alignment" "-O0" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp" "-o" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp"
# command output:
   Creating library C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.lib and object C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.exp

$ "C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp"
$ "FileCheck" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp" "--implicit-check-not= assumption " "--implicit-check-not=note:" "--implicit-check-not=error:"
$ ":" "RUN: at line 2"
$ "C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe" "-x" "c" "-fsanitize=alignment" "-O1" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp" "-o" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp"
# command output:
   Creating library C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.lib and object C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.exp

$ "C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp"
$ "FileCheck" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp" "--implicit-check-not= assumption " "--implicit-check-not=note:" "--implicit-check-not=error:"
$ ":" "RUN: at line 3"
$ "C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe" "-x" "c" "-fsanitize=alignment" "-O2" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp" "-o" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp"
# command output:
   Creating library C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.lib and object C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.exp

$ "C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp"
$ "FileCheck" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp" "--implicit-check-not= assumption " "--implicit-check-not=note:" "--implicit-check-not=error:"
$ ":" "RUN: at line 4"
$ "C:/b/rr/tmpeazzxh/w/src/third_party/llvm-build/Release+Asserts/./bin/clang.exe" "-x" "c" "-fsanitize=alignment" "-O3" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm\projects\compiler-rt\test\ubsan\TestCases\Pointer\alignment-assumption-attribute-assume_aligned-on-function.cpp" "-o" "C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp"
# command output:
LINK : fatal error LNK1104: cannot open file 'C:\b\rr\tmpeazzxh\w\src\third_party\llvm-build\Release+Asserts\projects\compiler-rt\test\ubsan\Standalone-x86_64\TestCases\Pointer\Output\alignment-assumption-attribute-assume_aligned-on-function.cpp.tmp'

# command stderr:
clang: error: linker command failed with exit code 1104 (use -v to see invocation)

error: command failed with exit status: 1104
Herald added a project: Restricted Project. · View Herald TranscriptFeb 5 2019, 8:30 AM
thakis added a comment.Feb 6 2019, 5:12 AM

I filed https://bugs.llvm.org/show_bug.cgi?id=40627 about the ubsan test flakes. It might not be related to this change, since several other tests that haven't been touched in a while also show these problems.

I filed https://bugs.llvm.org/show_bug.cgi?id=40627 about the ubsan test flakes. It might not be related to this change, since several other tests that haven't been touched in a while also show these problems.

Thank you.