LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 51816 - std::basic_string misbehavior when sizeof(CharT) > 8
Summary: std::basic_string misbehavior when sizeof(CharT) > 8
Status: NEW
Alias: None
Product: libc++
Classification: Unclassified
Component: All Bugs (show other bugs)
Version: unspecified
Hardware: PC Windows NT
: P enhancement
Assignee: Unassigned Clang Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-10 05:41 PDT by Yuriy Chernyshov
Modified: 2021-09-10 05:44 PDT (History)
2 users (show)

See Also:
Fixed By Commit(s):


Attachments
Test using gtest (1.94 KB, text/x-csrc)
2021-09-10 05:41 PDT, Yuriy Chernyshov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yuriy Chernyshov 2021-09-10 05:41:02 PDT
Created attachment 25247 [details]
Test using gtest

An attempt to compile std::basic_string with various weird character types would lead to a multiple issues.

So far I have found the following:

1. stack allocation size would grow due to overallocation, i. e:

if constexpr (sizeof(Char) == 12) {
    EXPECT_EQ(sizeof(s), 40u);
}

2. allocations would become power-of-two-exponential, i. e:
  
for (size_t i = 0; i < 47u; ++i) {
    s.push_back({});
}

if constexpr (sizeof(Char) <= 4) {
	// OK, do not request power-of-two blocks from allocator 
	EXPECT_EQ(
		s.capacity(),
		// capacity grows exponentially starting from 22 bytes
		47u
	);
} else if constexpr (sizeof(Char) == 8) {
	// Bad 
	EXPECT_EQ(
		s.capacity(),
		// capacity grows exponentially as the power-of-two
		63u
	);
}

3. Memory leak is detected by ASan when both -fsized-deallocation is enabled and stable ABI is used (i. e. _LIBCPP_ABI_VERSION == 2):

==1260518==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x6080000004a0 in thread T0:
  object passed to delete has wrong type:
  size of the allocated type:   84 bytes;
  size of the deallocated type: 72 bytes.
    #0 0x4cdc02 in void std::__y1::__libcpp_operator_delete<void*, unsigned long>(void*, unsigned long) /home/thegeorg/arcadia/contrib/libs/cxxsupp/libcxx/include/new:245:3
    #1 0x4cdc02 in void std::__y1::__do_deallocate_handle_size<>(void*, unsigned long) include/new:271:10
    #2 0x4cdc02 in std::__y1::__libcpp_deallocate(void*, unsigned long, unsigned long) include/new:285:14
    #3 0x4cdc02 in std::__y1::allocator<WeirdChar12>::deallocate(WeirdChar12*, unsigned long) include/memory:850:13
    #4 0x4cdc02 in std::__y1::allocator_traits<Allocator<WeirdChar12> >::deallocate(Allocator<WeirdChar12>&, WeirdChar12*, unsigned long) include/__memory/allocator_traits.h:484:14

This is caused by incorrect alignment in __recommend() and attempt to allocate odd number of characters. String capacity() must be even as odd bit will be overwritten by __long_mask.

Though C++ standard does not limit sizeof(CharT), I suggest static_assert(sizeof(CharT) <= 4) to be added into libc++.
Though this could break existing codebases, I consider the break fast to be the better option.

I do not think SSO is anyway helpful whenever sizeof(CharT) >= 8. Maybe we should ensure that std::basic_strin is always long is such cases.
Comment 1 Yuriy Chernyshov 2021-09-10 05:44:58 PDT
Stable ABI should be 

> i. e. _LIBCPP_ABI_VERSION == 1

of course