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

Add FreeBSD support #24

Merged
merged 16 commits into from
Apr 4, 2020
Merged

Add FreeBSD support #24

merged 16 commits into from
Apr 4, 2020

Conversation

valpackett
Copy link
Contributor

Wasn't that hard to port, I even got the injection mode running :)

  • had to initialize hooks::calloc very early to prevent infinite recursion
  • had to flock a separate lockfile because it's not currently implemented for fifos (I'll try fixing that in the kernel, seems odd, but we have to deal with that on released versions anyway)

This file doesn't seem to use Linux-specific features like mallinfo,
so remove the use of the deprecated-by-POSIX header (which causes
a hard error on FreeBSD 13).
Nobody expects this to build on a system without stdint.h,
so let's just define this unconditionally.
This is not currently defined on FreeBSD.
@milianw
Copy link
Collaborator

milianw commented Apr 1, 2020

woha, awesome! I'll try to find time for a proper review later this week. if you don't hear back, please nag :)

cheers

Copy link
Collaborator

@milianw milianw left a comment

Choose a reason for hiding this comment

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

lgtm in the heaptrack context, but could you please also upstream this to libbacktrace? see: github.com:ianlancetaylor/libbacktrace.git

@milianw
Copy link
Collaborator

milianw commented Apr 4, 2020

whops, please don't merge this yet, this comment was meant for a single commit: 52fbb4f

Copy link
Collaborator

@milianw milianw left a comment

Choose a reason for hiding this comment

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

excellent work overall! I think only the leaked extra lock file should be resolved, otherwise lgtm

thanks a lot!

3rdparty/libbacktrace/dwarf.c Show resolved Hide resolved
src/track/heaptrack.sh.cmake Show resolved Hide resolved
src/track/heaptrack.sh.cmake Outdated Show resolved Hide resolved
src/track/heaptrack.sh.cmake Show resolved Hide resolved
src/track/heaptrack.sh.cmake Outdated Show resolved Hide resolved
src/track/heaptrack.sh.cmake Outdated Show resolved Hide resolved
src/track/libheaptrack.cpp Show resolved Hide resolved
heaptrack_init locks a C++ mutex, which on FreeBSD calls libpthread
initialization, which calls calloc, and if the hook isn't set yet,
it recurses infinitely.
This is a hard error in clang, e.g.:

call_libc.cpp:2:5: error: exception specification in declaration does not match previous declaration
int puts(const char *str) noexcept;
    ^
/usr/include/stdio.h:279:6: note: previous declaration is here
int      puts(const char *);
         ^
NOTE: 'using namespace std' is not good - 'thread' from FreeBSD headers collides with std::thread.
@valpackett
Copy link
Contributor Author

Updated the script. Also, changed the noexcept thing because it's actually about compilers, not libcs, see commit message. (Why did I think that glibc would have an ifdef __cplusplus to make functions noexcept? :D)

@@ -175,7 +177,15 @@ extern "C" {

/// TODO: memalign, pvalloc, ...?

void* malloc(size_t size) noexcept
// NOTE: adding noexcept to C functions is a hard error in clang++
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should probably remove them altogethen then, but I can do that myself in a follow up

Copy link
Collaborator

@milianw milianw left a comment

Choose a reason for hiding this comment

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

great work, thanks a lot

@milianw milianw merged commit 3d62891 into KDE:master Apr 4, 2020
uqs pushed a commit to freebsd/freebsd-ports that referenced this pull request May 17, 2020
- Now contains actual heaptrack suport provded by GregV in [1]

[1] KDE/heaptrack#24

PR:		246131


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@535545 35697150-7ecd-e111-bb59-0022644237b5
uqs pushed a commit to freebsd/freebsd-ports that referenced this pull request May 17, 2020
- Now contains actual heaptrack suport provded by GregV in [1]

[1] KDE/heaptrack#24

PR:		246131
Jehops pushed a commit to Jehops/freebsd-ports-legacy that referenced this pull request May 18, 2020
- Now contains actual heaptrack suport provded by GregV in [1]

[1] KDE/heaptrack#24

PR:		246131


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@535545 35697150-7ecd-e111-bb59-0022644237b5
@adriaandegroot
Copy link
Contributor

This caused build fallout eventually on i386 because Elf32_Sxword isn't defined by the system headers. That's freebsd/freebsd-ports-kde@625db79 in the ports tree. @milianw I didn't submit that on Invent in the first place because it looks a bit grotty -- FreeBSD #ifdeffery . Anyway, in packaging we carry that patch.

@milianw
Copy link
Collaborator

milianw commented Jun 3, 2020

@adriaandegroot please upstream

kdesysadmin pushed a commit that referenced this pull request Sep 4, 2023
Return when we get asked for invalid data which seems to happen
through the KDChart attributes model. Fixes assertions/crashes like:

```
#0  0x00007fdcfc88e83c in  () at /usr/lib/libc.so.6
#1  0x00007fdcfc83e668 in raise () at /usr/lib/libc.so.6
#2  0x00007fdcfc8264b8 in abort () at /usr/lib/libc.so.6
#3  0x00007fdcfd8a0098 in qt_assert(char const*, char const*, int) () at /usr/lib/libQt5Core.so.5
#4  0x00007fdcfd8a0157 in  () at /usr/lib/libQt5Core.so.5
#5  0x00005649da68cf3f in QVector<QBrush>::at(int) const (this=0x6070001170a0, i=-1) at /usr/include/qt/QtCore/qvector.h:449
#6  0x00005649da70272e in ChartModel::headerData(int, Qt::Orientation, int) const (this=0x607000117060, section=-1, orientation=Qt::Horizontal, role=175763350) at /home/milian/projects/src/heaptrack/src/analyze/gui/chartmodel.cpp:69
#7  0x00007fdcfda67f5e in QAbstractProxyModel::headerData(int, Qt::Orientation, int) const () at /usr/lib/libQt5Core.so.5
#8  0x00007fdcfda7ba54 in QSortFilterProxyModel::headerData(int, Qt::Orientation, int) const () at /usr/lib/libQt5Core.so.5
#9  0x00007fdcff9b6dbf in KChart::AttributesModel::headerData(int, Qt::Orientation, int) const () at /usr/lib/libKChart.so.2
#10 0x00007fdcff9af76a in  () at /usr/lib/libKChart.so.2
#11 0x00007fdcff9a7605 in KChart::AbstractDiagram::brush(int) const () at /usr/lib/libKChart.so.2
#12 0x00007fdcff9ac495 in KChart::AbstractDiagram::datasetBrushes() const () at /usr/lib/libKChart.so.2
#13 0x00007fdcff9ccd77 in  () at /usr/lib/libKChart.so.2
#14 0x00007fdcff9cda4b in KChart::Legend::buildLegend() () at /usr/lib/libKChart.so.2
#15 0x00007fdcff9ce4ba in KChart::Legend::setNeedRebuild() () at /usr/lib/libKChart.so.2
#16 0x00007fdcfdad17f3 in  () at /usr/lib/libQt5Core.so.5
#17 0x00007fdcff98c4f3 in KChart::DiagramObserver::diagramDataChanged(KChart::AbstractDiagram*) () at /usr/lib/libKChart.so.2
#18 0x00007fdcfdad17f3 in  () at /usr/lib/libQt5Core.so.5
#19 0x00007fdcfda59182 in QAbstractItemModel::modelReset(QAbstractItemModel::QPrivateSignal) () at /usr/lib/libQt5Core.so.5
#20 0x00007fdcfda8506d in  () at /usr/lib/libQt5Core.so.5
#21 0x00007fdcfdad17f3 in  () at /usr/lib/libQt5Core.so.5
#22 0x00007fdcfda59182 in QAbstractItemModel::modelReset(QAbstractItemModel::QPrivateSignal) () at /usr/lib/libQt5Core.so.5
#23 0x00005649da708068 in ChartModel::resetData(ChartData const&) (this=0x607000117060, data=...) at /home/milian/projects/src/heaptrack/src/analyze/gui/chartmodel.cpp:261
#24 0x00005649da569a5d in operator()(ChartData const&) const (__closure=0x604000258e20, data=...) at /home/milian/projects/src/heaptrack/src/analyze/gui/mainwindow.cpp:223
```

BUG: 473634
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.

3 participants