Comments (13)
Setting priority to high, as Chromium can't use CLD2 on Android or (presumably)
iOS until this is resolved.
Original comment by [email protected]
on 20 Mar 2014 at 3:12
- Added labels: Priority-High
- Removed labels: Priority-Medium
from cld2.
PS, we should fix this for 64-bit while we're here, so replace "%4" in my
previous comment with "%8".
Original comment by [email protected]
on 20 Mar 2014 at 3:12
from cld2.
For posterity:
$ cat /proc/cpuinfo | grep ARM
Processor : ARMv7 Processor rev 10 (v7l)
I'm somewhat surprised by this, because the comments in port.h seem to imply
that ARMv7+ can do unaligned reads but they're just slow. Not the case
apparently, as the SIGBUS came from an ARM7 device. Anyhow, I guess we're not
using port.h here, and if we were it would (irritatingly) probably be broken as
well. But theoretically we could do UNALIGNED_LOAD32 for this as a short-term
fix while we get the right thing done. WDYT?
Original comment by [email protected]
on 20 Mar 2014 at 3:25
from cld2.
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CIHCGCFD.ht
ml
"To enable unaligned support, you must:
Clear the A bit, bit 1, of CP15 register 1 in your initialization code.
Set the U bit, bit 22, of CP15 register 1 in your initialization code."
Original comment by [email protected]
on 20 Mar 2014 at 3:33
from cld2.
$ grep -R -5 "reinterpret_cast" . | grep -5 int32 >
~/Desktop/unaligned_landmines.txt
Original comment by [email protected]
on 20 Mar 2014 at 3:34
Attachments:
from cld2.
."/cldutil_shared.cc-// OVERSHOOTS up to 3 bytes"
What could possibly go wrong?
Original comment by [email protected]
on 20 Mar 2014 at 3:38
from cld2.
It looks to me like we need to make repairs in (at least) these spots:
1. UTF8GenericScan in utf8statetable.cc
2. UTF8GenericScanFastAscii in utf8statetable.cc
3. BiHashV2 in cldutil_shared.cc
4. QuadHashV2Mix in cldutil_shared.cc
5. OctaHash40Mix in cldutil_shared.cc
The data loader has the same problem, but it should be immune because the
tables that are being loaded are already 64-bit aligned by the data extractor.
Hooray, forward thinking (thanks, Dick!). Wouldn't hurt to drop a note in
CLD2DynamicDataLoader::loadDataInternal to this effect, though. Dick, do you
see any other spots that need repairs?
Original comment by [email protected]
on 20 Mar 2014 at 3:41
from cld2.
Here's a proposed patch that fixes #1. I've tested this on ARM, and it works.
The unit tests pass, and I threw in some logging (which I took out before
generating the diff) that shows that the unit tests as-is very thoroughly
execute the range of values 0-7 as initial offsets. Hooray! This seems
reasonable to me, but Dick, PTAL.
Original comment by [email protected]
on 21 Mar 2014 at 6:09
Attachments:
from cld2.
Checking on the remaining functions:
1. UTF8GenericScan in utf8statetable.cc
Fixed by the patch posted here.
2. UTF8GenericScanFastAscii in utf8statetable.cc
Not called in any code that I can see. Can we just delete this?
3. BiHashV2 in cldutil_shared.cc
Uses the UNALIGNED_LOAD macro, and should therefore not have a problem.
4. QuadHashV2Mix in cldutil_shared.cc
Also uses the UNALIGNED_LOAD macro, and should therefore not have a problem.
5. OctaHash40Mix in cldutil_shared.cc
Also uses the UNALIGNED_LOAD macro, and should therefore not have a problem.
So I think all we actually need is the patch I've posted here, or a better
equivalent. Dick, does this sound good?
Original comment by [email protected]
on 21 Mar 2014 at 7:36
from cld2.
I had a long discussion offline with several subject matter experts about ARM7+
support for unaligned access, and the takeaway was that under certain
circumstances unaligned access on ARM will cause a SIGBUS or other issues even
if unaligned access is supported and enabled. Their recommendation was that NO
code should rely upon unaligned access if it's going to run on ARM.
The rest of the code here uses the UNALIGNED_LOAD but it's my impression from
the comments in utf8statetable.cc that we need to be fast, so I see no obvious
solution to fix this other than the proposed patch.
Original comment by [email protected]
on 27 Mar 2014 at 10:57
from cld2.
Dick made a fix to use UNALIGNED_LOAD:
https://code.google.com/p/cld2/source/detail?r=158
Sadly this doens't work. I believe the reason is that port.h incorrectly
assumes that we don't need aligned loads on ARM7:
https://code.google.com/p/cld2/source/browse/trunk/internal/port.h#57
The crash still occurs in exactly the same place with r158. We either need to
fix port.h or apply my patch to this method (and the others using
UNALIGNED_LOAD).
Original comment by [email protected]
on 2 Apr 2014 at 3:43
from cld2.
Here's a patch that fixes the problem for ARM. I'm dubious that we should be
doing this at all; we should probably just turn off unaligned access for all of
ARM and be done with it. Subsequently we can investigate the possibility of
incrementally killing off the use of UNALIGNED_LOAD in any critical-path code.
Original comment by [email protected]
on 2 Apr 2014 at 3:58
Attachments:
from cld2.
port.h patch submitted as r159 after consultation with Dick.
Original comment by [email protected]
on 3 Apr 2014 at 7:17
- Changed state: Fixed
from cld2.
Related Issues (20)
- CLD2 result chunk vector omits portions of input file HOT 6
- Dynamic data loading should not use iostream HOT 5
- Windows build fails: undeclared identifier 'close' HOT 6
- Support mmap-ing dynamic data on win32 HOT 5
- Build warning on Windows with clang HOT 2
- Eliminate redundancy and/or simplify default case for compiling unittest_data.h HOT 4
- Missing include in cld2_dynamic_data_loader.cc HOT 1
- cld2_dynamic_data.cc and cld2_dynamic_data_loader.cc problems on Win32 HOT 10
- Enable dynamic data for 20141015 release HOT 1
- New GCC 5.0 hits problem with narrowing in list-initializers HOT 2
- CLD should check result of "new" in all use cases HOT 1
- please use CFLAGS CXXFLAGS CPPFLAGS and LDFLAGS HOT 3
- please provide a SONAME HOT 13
- cld2 testsuite failures HOT 3
- Compilation issues in Visual Studio
- Compilation failure on VS2015 on Windows
- Check in tools for generating generated_* files HOT 2
- Add armv8-a support HOT 5
- new code location? HOT 6
- Add possibility to set MinReliableKeepPercent HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from cld2.