Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

static_cast() of sentinel value in LLVM intrusive lists is Undefined Behavior #27127

Closed
bogner opened this issue Feb 27, 2016 · 6 comments
Closed
Assignees
Labels
bugzilla Issues migrated from bugzilla

Comments

@bogner
Copy link
Contributor

bogner commented Feb 27, 2016

Bugzilla Link 26753
Resolution FIXED
Resolved on Aug 17, 2016 18:37
Version trunk
OS All

Extended Description

Many of our iplist classes (MachineBasicBlock, MemoryAccess) do something like this:

template <class T> ListNode { T *prev, *next; };
template <class T> struct List {
  ListNode<T> Sentinel;
  T *Head = static_cast<T *>(&Sentinel);
  void ensureHead() {}
};

Because of iterators have pointers to T instead of ListNodeBase, this exposes UB.

See here for more details: http://lists.llvm.org/pipermail/llvm-dev/2015-October/091115.html

@bogner
Copy link
Contributor Author

bogner commented Feb 27, 2016

assigned to @dexonsmith

@dexonsmith
Copy link
Mannequin

dexonsmith mannequin commented Feb 27, 2016

Just as an update since my last email, I have a plan for fixing this, but it's not safe until I've made the implicit conversion in MachineInstrBundleIterator explicit:

template struct MachineInstrBundleIterator {
ilist_iterator I;
operator NodeTy*() { return I.getNodePtrUnchecked(); }
};

My fix will remove getNodePtrUnchecked entirely from ilist_iterator, and this code will change to:

{ return &*I; }

so I need to make sure all callers have a valid iterator.

@dexonsmith
Copy link
Mannequin

dexonsmith mannequin commented Jul 27, 2016

r276902

Just as an update since my last email, I have a plan for fixing this, but
it's not safe until I've made the implicit conversion in
MachineInstrBundleIterator explicit...

Finished that in r276902. I'll try figure out what's left when I'm back from vacation.

@dexonsmith
Copy link
Mannequin

dexonsmith mannequin commented Aug 11, 2016

I have patches locally that remove the inherent UB from iplist/ilist/ilist_node/ilist_iterator, with all tests passing (a few fixes to end()-dereferences required). Shouldn't be long now?

@dexonsmith
Copy link
Mannequin

dexonsmith mannequin commented Aug 12, 2016

Just sent a review to the list to fix this.
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160808/381741.html

@dexonsmith
Copy link
Mannequin

dexonsmith mannequin commented Aug 18, 2016

A few follow-ups to do for cleanup, but this was fixed in r278974!

I can't imagine anyone needs/wants to cherry-pick all of these (... just re-branch), but since I have them, this was:
r249439 = dd076fe
r249469 = 67f918e
r249473 = ac3b52b
r249476 = 3c05041
r249480 = 0ea7b26
r249602 = 2a8bcfa
r249615 = 10b15bc
r249758 = 9dd9ad6
r249763 = ec0b29e
r249764 = 9c2fb73
r249767 = e38e995
r249782 = eac3095
r249783 = 4100fc0
r249849 = 8d79660263017f344bf01f6af7e488718d42061f (clang)
r249850 = 42247d0
r249851 = 9cb8e12
r249852 = 969c1a3
r249867 = aa464da
r249869 = 7e15cfe
r249875 = 030cf8e
r249876 = 8d36360
r249877 = d578a48
r249879 = 3f2c43f
r249880 = b1510c2
r249883 = 1b44cbf
r249884 = 9731c60
r249901 = ba91007
r249903 = fdec461
r249915 = ac4d7b6
r249922 = 5d8fcb9
r249925 = d3a5adc
r250142 = 5270f88
r250144 = f25f287
r250181 = bcd41c0
r250183 = f83f208
r250185 = ad48b01
r250186 = 210a154
r250187 = 1e2bc42
r250192 = fb34e26
r250193 = cf8b618
r250197 = 7e8a22b
r250211 = b886fde
r250214 = 20f1c08
r250216 = 970ba66
r250218 = 3d5dddd
r250741 = d765f75
r250745 = f792618
r250748 = 989a95d
r250756 = 25622c1
r250759 = 974314a
r250765 = 2b76674
r250766 = 7a225f7
r250769 = ddeb82c
r250776 = 97fa27f
r250778 = 8525f3a
r250779 = db991cb
r250781 = 2b14990
r250787 = fa3f538
r250788 = a1a3f2d
r250790 = e8e219b
r250791 = 090db02
r250792 = 3698b0e
r250842 = 00f8d5f
r250843 = 70f8c20
r250852 = 1e221d5
r252357 = 1583cbbc91e4289536bb9ec8e2f064c4d784b897 (polly)
r252358 = 2ceddccf8fd4a21649214eccf493fc8fa60760c0 (clang)
r252360 = 67fafbb08d456c44dabca62e3af4c20abb715d65 (clang)
r252371 = 5013c51
r252372 = 84cb897
r252373 = d81c552 (revert r252372)
r252378 = b377de5cefab24ad523821dfee3ce31c3c7a8dbb (lldb)
r252379 = 7d7668e
r252380 = 45d3f0d
r252538 = 9df84ef
r252694 = 6426aea
r261491 = 7292344
r261492 = be77f42
r261493 = 2d6fcba
r261495 = 14147ed
r261497 = d73d33f
r261498 = 8de6150
r261499 = 7b83cb6
r261500 = 151a6ae
r261502 = b64deed
r261504 = 6e5736e
r261507 = 17ef676
r261508 = d2c0ad9
r261509 = 8c34e06
r261510 = 01f8c42 (revert r261509)
r261511 = 65b18dd
r261567 = 20a6252 (revert r261504)
r261577 = 110284e (revert r261504 p2)
r261605 = 5b9b80e
r262115 = 42e1835
r262116 = e71d87d
r262118 = 6667d85
r262141 = 63ec7f0
r262142 = 0ce039d
r262143 = 6666292
r262144 = d7b3cd7
r262145 = a26cd9c
r262146 = 7dd408d
r262149 = 1d75c8d
r262151 = 5144d35
r262152 = cc3610d
r262154 = 88bff1c
r274189 = 567409d
r274193 = e1a95d0
r274287 = a204da2
r274290 = e475645
r274292 = b6b73f6
r274294 = 8a1fda1
r274295 = 6feb5c9
r274297 = 14a454a
r274298 = a076358
r274300 = fdab22c
r274301 = 03941cc
r274303 = dbcbdf3
r274304 = a354e21
r274310 = 5bcaf78
r274311 = ed5a96f
r274315 = e16c630
r274317 = 1b7dbea
r274319 = 4383a51
r274353 = ce5fdc0
r274354 = effa4cc
r274355 = 2cafa02
r274360 = f634f61
r274361 = 1032b93
r274363 = 7aad63c
r274888 = c3d3821
r274892 = fccaf97
r274893 = ccb7a2f
r274898 = 04671b9
r274899 = cc2c87f
r274902 = f360e3f
r274903 = a60f808
r274904 = 9ea0ae7
r274906 = 83b2ab7
r274907 = 86a3f78
r274911 = 5de74a6
r274912 = a3dcb9e
r274913 = 35be20e
r274920 = c347f0a
r274931 = 0795ebd
r274933 = 3f1460a
r274942 = ccc4444
r275137 = cf91a6f
r275141 = 721fe5d
r275142 = 8c13ec2
r275149 = a17edeb
r276864 = f5be002
r276899 = cfc6fb4
r276902 = fe912bf
r278344 = 91c138b
r278346 = 9040acb
r278347 = 8949cc2
r278355 = aba9f58
r278394 = eeb1d4f
r278467 = 369a632
r278468 = b5efffb (lld)
r278478 = 21bcd75
r278513 = 3e97be2
r278521 = 9396e31 (lld)
r278523 = 8457dd7 (lld)
r278525 = 573f6313b0a82039b804bf0afad05f8fa3ef218e (clang)
r278532 = 6394023
r278537 = a33b92e
r278539 = f74d382
r278542 = 3d3a4a4
r278572 = 2b6735e
r278577 = be1a4e1 (thanks to hans@hanshq.net)
r278587 = 46b81ba (thanks to hans@hanshq.net)
r278858 = 52a07ee
r278870 = e3c3e55
r278872 = cbbc1ca
r278873 = 9e4c8a3
r278878 = f623792
r278879 = d4e0802
r278880 = 4a3738e
r278881 = 936a383
r278884 = 370879f
r278886 = 3b3dd88
r278887 = eb6a210
r278898 = 1802c44dd4ae1f9abb99ba7746c9eccdc9ef59c8 (clang)
r278974 = b2724ac

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

1 participant