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

Reduction of issue with PR implement URLPattern #785 #823

Closed
wants to merge 7 commits into from
Closed

Conversation

lemire
Copy link
Member

@lemire lemire commented Jan 3, 2025

There is an issue with GCC in PR #785

We can reproduce it with a simple case. This is the test:

#include "ada.h"



bool test() {
  auto tokenize_result = ada::url_pattern_helpers::tokenize();
  return tokenize_result.empty();
}

int main() {
    test();
    return EXIT_SUCCESS;
}

The tokenize function is declared like so:

#ifndef ADA_URL_PATTERN_HELPERS_H
#define ADA_URL_PATTERN_HELPERS_H

#include <vector>

namespace ada::url_pattern_helpers {
std::vector<int> tokenize();
}  // namespace ada::url_pattern_helpers
#endif

It is then defined like so:

#include "ada/url_pattern_helpers.h"

namespace ada::url_pattern_helpers {

std::vector<int> tokenize() {
  std::vector<int> token_list;
  return token_list;
}

}  // namespace ada::url_pattern_helpers

Running the test with sanitizers, I get...

$ ./simple_wpt_urlpattern_tests 
=================================================================
==2613422==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f10f1d00038 at pc 0x0000004eb025 bp 0x7fff461aab60 sp 0x7fff461aab50
WRITE of size 8 at 0x7f10f1d00038 thread T0
    #0 0x4eb024 in std::__cxx1998::_Vector_base<int, std::allocator<int> >::_Vector_impl_data::_Vector_impl_data() /opt/rh/gcc-toolset-12/root/usr/include/c++/12/bits/stl_vector.h:100
    #1 0x4dc487 in std::__cxx1998::_Vector_base<int, std::allocator<int> >::_Vector_impl::_Vector_impl() /opt/rh/gcc-toolset-12/root/usr/include/c++/12/bits/stl_vector.h:139
    #2 0x494679 in std::__cxx1998::_Vector_base<int, std::allocator<int> >::_Vector_base() /opt/rh/gcc-toolset-12/root/usr/include/c++/12/bits/stl_vector.h:312
    #3 0x494695 in std::__cxx1998::vector<int, std::allocator<int> >::vector() /opt/rh/gcc-toolset-12/root/usr/include/c++/12/bits/stl_vector.h:526
    #4 0x4946c1 in std::__debug::vector<int, std::allocator<int> >::vector() /opt/rh/gcc-toolset-12/root/usr/include/c++/12/debug/vector:172
    #5 0x43b33b in ada::url_pattern_helpers::tokenize() /home/dlemire/CVS/github/ada/src/url_pattern_helpers.cpp:6
    #6 0x4057ed in test() /home/dlemire/CVS/github/ada/tests/simple_wpt_urlpattern_tests.cpp:8
    #7 0x405871 in main /home/dlemire/CVS/github/ada/tests/simple_wpt_urlpattern_tests.cpp:13
    #8 0x7f10f3a295cf in __libc_start_call_main (/lib64/libc.so.6+0x295cf) (BuildId: d78a44ae94f1d320342e0ff6c2315b2b589063f8)
    #9 0x7f10f3a2967f in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2967f) (BuildId: d78a44ae94f1d320342e0ff6c2315b2b589063f8)
    #10 0x405634 in _start (/mnt/bigdisk/lemire/github/ada/buildsani/tests/simple_wpt_urlpattern_tests+0x405634) (BuildId: 63c849374513aff6d6749b6abde45b3df2b572f1)

Address 0x7f10f1d00038 is located in stack of thread T0 at offset 56 in frame
    #0 0x405705 in test() /home/dlemire/CVS/github/ada/tests/simple_wpt_urlpattern_tests.cpp:7

  This frame has 1 object(s):
    [32, 56) 'tokenize_result' (line 8) <== Memory access at offset 56 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /opt/rh/gcc-toolset-12/root/usr/include/c++/12/bits/stl_vector.h:100 in std::__cxx1998::_Vector_base<int, std::allocator<int> >::_Vector_impl_data::_Vector_impl_data()
Shadow bytes around the buggy address:
  0x7f10f1cffd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f10f1cffe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f10f1cffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f10f1cfff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f10f1cfff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x7f10f1d00000: f1 f1 f1 f1 00 00 00[f3]f3 f3 f3 f3 00 00 00 00
  0x7f10f1d00080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f10f1d00100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f10f1d00180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f10f1d00200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f10f1d00280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==2613422==ABORTING

@anonrig anonrig changed the base branch from main to yagiz/add-url-pattern January 3, 2025 18:19
@anonrig anonrig changed the base branch from yagiz/add-url-pattern to main January 3, 2025 18:20
@lemire lemire marked this pull request as draft January 3, 2025 18:20

std::vector<int> tokenize() {
std::vector<int> token_list;
return token_list;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if you return std::move(token_list) here? @jasnell's recommendation

@lemire
Copy link
Member Author

lemire commented Jan 3, 2025

Solved by @guidovranken and @pdimov

Screenshot 2025-01-03 at 2 11 01 PM

@lemire lemire closed this Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants