With the release of pocl 0.14 it is now compatible with llvm39 and llvm40 so with this update the port now defaults to 40 and will follow MESA_LLVM_VER if set. The port switches to using CMake because that is what this release supports; there are no autotools files in 0.14. Also, i386 has been unblocked as it appears to work and upstream appears to support even non-x86. Maintainer timed out in PR 218332.
Details
Details
- Reviewers
swills feld jbeich - Commits
- rP440691: Update to 0.14 and switch to llvm40 by default
Poudriere 11.0 amd64/i386 OK
Diff Detail
Diff Detail
- Repository
- rP FreeBSD ports repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| lang/pocl/Makefile | ||
|---|---|---|
| 37 ↗ | (On Diff #27968) | Did you forget to drop no longer used DEBUG option? |
| lang/pocl/files/patch-lib_CL_devices_cpuinfo.c | ||
| 21 ↗ | (On Diff #27968) | Prepend defined(__DragonFly__) || as hw.clockrate is also supported there. OpenBSD can use HW_CPUSPEED instead but out of scope for now. pocl-0.13 builds fine on DragonFly without patches. Let's keep pocl-0.14 the same way: |
| 51 ↗ | (On Diff #27968) | Replace with #else that calls sysconf(_SC_NPROCESSORS_ONLN) instead. All BSDs (and OS X) support hw.ncpu. However, on NetBSD hw.ncpu is different from hw.ncpuonline. https://github.com/DragonFlyBSD/DragonFlyBSD/blob/6f1d2f41495d/lib/libc/gen/sysconf.c#L629 |
| lang/pocl/files/patch-lib_CL_devices_cpuinfo.c | ||
|---|---|---|
| 80 ↗ | (On Diff #27968) | Maybe fill device->long_name via HW_MODEL sysctl on all BSDs. |
Comment Actions
Revise patch-lib_CL_devices_cpuinfo.c
| lang/pocl/Makefile | ||
|---|---|---|
| 37 ↗ | (On Diff #27968) | Cmake responds to the DEBUG option without any extra setting. Try the option and notice it builds Debug instead of Release configuration. |
| lang/pocl/files/patch-lib_CL_devices_cpuinfo.c | ||
| 21 ↗ | (On Diff #27968) | I had just quickly changed preprocessor directives initially. Going back and looking at the code within, there's probably a more accurate answer under dev.cpu than what hw.clockrate offers but I do not know how portable that would be. |
Comment Actions
I'm a bit biased against DEBUG but the rest looks fine.
| lang/pocl/Makefile | ||
|---|---|---|
| 37 ↗ | (On Diff #27968) | Does it provide anything more than symbols (e.g. assertions, statistics, verbosity, slow path)? Otherwise, WITH_DEBUG=1 facilitiy is generic for every port that respects CFLAGS/STRIP. To get useful stacktraces one still needs to build dependencies with symbols. And default flags may not be enough e.g., crashes may disappear without -O level or be confusing without -fsanitize=address. |
Comment Actions
I had left DEBUG in place because it was there and was doing something but if it shouldn't be there then DEBUG can easily go; WITH_DEBUG=1 does the same thing as setting the option.
| lang/pocl/files/patch-lib_CL_devices_cpuinfo.c | ||
|---|---|---|
| 8 ↗ | (On Diff #28026) | Windows and Solaris lack <sys/sysctl.h>, so when upstreaming you may need include guards. # CMakeLists.txt include(CheckIncludeFiles) check_include_files("sys/types.h;sys/sysctl.h" HAVE_SYS_SYSCTL_H) /* config.h.in.cmake */ #cmakedefine HAVE_SYS_SYSCTL_H /* lib/CL/devices/cpuinfo.c */ #ifdef HAVE_SYS_SYSCTL_H #include <sys/types.h> #include <sys/sysctl.h> #endif |
| 42 ↗ | (On Diff #28026) | value string seems to lack terminating NUL character. $ cc -g -fsanitize=address test.c $ ./a.out ================================================================= ==17273==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60f0000000ed at pc 0x00000042d769 bp 0x7fffffffde30 sp 0x7fffffffd5d8 READ of size 174 at 0x60f0000000ed thread T0 #0 0x42d768 in __interceptor_strlen (/home/foo/a.out+0x42d768) #1 0x80125a314 in vsscanf_l /usr/src/lib/libc/stdio/vsscanf.c:69:23 #2 0x80125a314 in vsscanf /usr/src/lib/libc/stdio/vsscanf.c:77 #3 0x43d24d in sscanf (/home/foo/a.out+0x43d24d) #4 0x499bba in pocl_cpuinfo_detect_max_clock_frequency /home/foo/test.c:26:7 #5 0x499cfa in main /home/foo/test.c:40:18 #6 0x40d1af in _start /usr/src/lib/csu/amd64/crt1.c:72:7 0x60f0000000ed is located 0 bytes to the right of 173-byte region [0x60f000000040,0x60f0000000ed) allocated by thread T0 here: #0 0x48555c in malloc (/home/foo/a.out+0x48555c) #1 0x499b5e in pocl_cpuinfo_detect_max_clock_frequency /home/foo/test.c:23:23 #2 0x499cfa in main /home/foo/test.c:40:18 #3 0x40d1af in _start /usr/src/lib/csu/amd64/crt1.c:72:7 #4 0x8006ddfff (<unknown module>) SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/foo/a.out+0x42d768) in __interceptor_strlen Shadow bytes around the buggy address: 0x4c1dffffffc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x4c1dffffffd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x4c1dffffffe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x4c1dfffffff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x4c1e00000000: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 =>0x4c1e00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00[05]fa fa 0x4c1e00000020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x4c1e00000030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x4c1e00000040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x4c1e00000050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x4c1e00000060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 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 ==17273==ABORTING |
| 91 ↗ | (On Diff #28026) | #elif defined(HW_MODEL) and/or #elif defined(HAVE_SYS_SYSCTL_H) (see another comment) maybe less assuming about portability. |
| lang/pocl/files/patch-lib_CL_devices_cpuinfo.c | ||
|---|---|---|
| 8 ↗ | (On Diff #28026) | If you do this move #ifdef HAVE_SYS_SYSCTL_H block after #include "config.h". ;) |
| lang/pocl/files/patch-lib_CL_devices_cpuinfo.c | ||
|---|---|---|
| 8 ↗ | (On Diff #28026) | I noticed the MSVC check but this file looks to be rather Linux specific so I had not thought about portability beyond there, not that I had put any thought to upstreaming, but easy enough to add those checks. |
| 42 ↗ | (On Diff #28026) | Well, that answers that question... sysctl(3) doesn't specifically address the NUL terminator but implies by example that it would be provided and accounted for in the size. On the other hand, I have seen manual NUL termination of strings from sysctl() often enough to be suspicious of the simplistic example. |